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 16:52:31 GMT
On Tue, Nov 22, 2016 at 6:48 PM, Stack <stack@duboce.net> wrote:

> 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).
>
>
Let me clarify the above. I want review addressed before merge happens.
Sorry if any confusion.
St.Ack






> 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