hbase-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stack <st...@duboce.net>
Subject Re: [DISCUSSION] Merge Backup / Restore - Branch HBASE-7912
Date Wed, 23 Nov 2016 02:48:47 GMT
On Tue, Nov 22, 2016 at 3:17 PM, Vladimir Rodionov <vladrodionov@gmail.com>
wrote:

> >> and/or he answered most of the review feedback
>
> No, questions are still open, but I do not see any blockers and we have
> HBASE-16940 to address these questions.
>
>
Agree. No blockers but stuff that should be dealt with (No one will pay me
any attention once merge goes in -- smile).

St.Ack



> On Tue, Nov 22, 2016 at 3:04 PM, Devaraj Das <ddas@hortonworks.com> wrote:
>
> > Hi Stack, hats off to you for spending so much time on this! Thanks! From
> > my understanding, Vlad has raised follow-up jiras for the issues you
> > raised, and/or he answered most of the review feedback. So, do you think
> we
> > could do a merge vote now?
> > Devaraj.
> > ________________________________________
> > From: Vladimir Rodionov <vladrodionov@gmail.com>
> > Sent: Monday, November 21, 2016 8:34 PM
> > To: dev@hbase.apache.org
> > Subject: Re: [DISCUSSION] Merge Backup / Restore - Branch HBASE-7912
> >
> > >> I have spent a good bit of time reviewing and testing this feature. I
> > would
> > >> like my review and concerns addressed and I'd like it to be clear how;
> > >> either explicit follow-on issues, pointers to where in the patch or
> doc
> > my
> > >> remarks have been catered to, etc. Until then, I am against commit.
> >
> > Stack, mega patch review comments will be addressed in the dedicated
> JIRA:
> > HBASE-16940
> > I have open several other JIRAs to address your other comments (not on
> > review board).
> >
> > Details are here (end of the thread):
> > https://issues.apache.org/jira/browse/HBASE-14123
> >
> > Let me know what else should we do to move merge forward.
> >
> > -Vlad
> >
> >
> > On Fri, Nov 18, 2016 at 4:54 PM, Stack <stack@duboce.net> wrote:
> >
> > > On Fri, Nov 18, 2016 at 3:53 PM, Ted Yu <yuzhihong@gmail.com> wrote:
> > >
> > > > Thanks, Matteo.
> > > >
> > > > bq. restore is not clear if given an incremental id it will do the
> full
> > > > restore from full up to that point or if i need to apply manually
> > > > everything
> > > >
> > > > The restore takes into consideration of the dependent backup(s).
> > > > So there is no need to apply preceding backup(s) manually.
> > > >
> > > >
> > > I ask this question on the issue. It is not clear from the usage or doc
> > how
> > > to run a restore from incremental. Can you fix in doc and usage how so
> I
> > > can be clear and try it. Currently I am stuck verifying a round trip
> > backup
> > > restore made of incrementals.
> > >
> > > Thanks,
> > > S
> > >
> > >
> > >
> > > > On Fri, Nov 18, 2016 at 3:48 PM, Matteo Bertozzi <
> > > theo.bertozzi@gmail.com>
> > > > wrote:
> > > >
> > > > > I did one last pass to the mega patch. I don't see anything major
> > that
> > > > > should block the merge.
> > > > >
> > > > > - most of the code is isolated in the backup package
> > > > > - all the backup code is client side
> > > > > - there are few changes to the server side, mainly for cleaners,
> wal
> > > > > rolling and similar (which is ok)
> > > > > - there is a good number of tests, and an integration test
> > > > >
> > > > > the code seems to have still some left overs from the old
> > > implementation,
> > > > > and some stuff needs a cleanup. but I don't think this should be
> used
> > > as
> > > > an
> > > > > argument to block the merge. I think the guys will keep working on
> > this
> > > > and
> > > > > they may also get help of others once the patch is in master.
> > > > >
> > > > > I still have my concerns about the current limitations, but these
> are
> > > > > things already planned for phase 3, so some of this stuff may even
> be
> > > in
> > > > > the final 2.0.
> > > > > but as long as we have a "current limitations" section in the user
> > > guide
> > > > > mentioning important stuff like the ones below, I'm ok with it.
> > > > >  - if you write to the table with Durability.SKIP_WALS your data
> will
> > > not
> > > > > be in the incremental-backup
> > > > >  - if you bulkload files that data will not be in the incremental
> > > backup
> > > > > (HBASE-14417)
> > > > >  - the incremental backup will not only contains the data of the
> > table
> > > > you
> > > > > specified but also the regions from other tables that are on the
> same
> > > set
> > > > > of RSs (HBASE-14141) ...maybe a note about security around this
> topic
> > > > >  - the incremental backup will not contains just the "latest row"
> > > between
> > > > > backup A and B, but it will also contains all the updates occurred
> in
> > > > > between. but the restore does not allow you to restore up to a
> > certain
> > > > > point in time, the restore will always be up to the "latest backup
> > > > point".
> > > > >  - you should limit the number of "incremental" up to N (or maybe
> > > SIZE),
> > > > to
> > > > > avoid replay time becoming the bottleneck. (HBASE-14135)
> > > > >
> > > > > I'll be ok even with the above not being in the final 2.0,
> > > > > but i'd like to see as blocker for the final 2.0 (not the merge)
> > > > >  - the backup code moved in an hbase-backup module
> > > > >  - and some more work around tools, especially to try to unify and
> > make
> > > > > simple the backup experience (simple example: in some case there
> is a
> > > > > backup_id argument in others a backupId argument. or things like..
> > > > restore
> > > > > is not clear if given an incremental id it will do the full restore
> > > from
> > > > > full up to that point or if i need to apply manually everything).
> > > > >
> > > > > in conclusion, I think we can open a merge vote. I'll be +1 on it,
> > and
> > > I
> > > > > think we should try to reject -1 with just a "code cleanup"
> > motivation,
> > > > > since there will still be work going on on the code after the
> merge.
> > > > >
> > > > > Matteo
> > > > >
> > > > >
> > > > > On Sun, Nov 6, 2016 at 10:54 PM, Devaraj Das <ddas@hortonworks.com
> >
> > > > wrote:
> > > > >
> > > > > > Stack and others, anything else on the patch? Merge to master
> now?
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message