You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2019/01/31 20:08:52 UTC
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12327
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
KUDU-2677 Implement new gflag for backup history retention
This patch implements a new flag,
--backup_max_age_sec, separate from --tablet_history_max_age_sec. Actual
retention will be maximum of these two flags. The purpose of this is to
smooth the user experience as backup evolves: in the absence of this
flag, for the initial implementation of backup, users will need to raise
--tablet_history_max_age_sec, and then likely lower it again when a
future release adds more backup capabilities or configuration behind the
scenes. With the new --backup_max_age_sec flag, users can just raise the
new flag, and ideally we will keep the semantics of "I want to be able
to back up to this point" in future releases, minimizing user-end
configuration changes.
Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
---
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tools/ksck_remote-test.cc
5 files changed, 96 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/12327/1
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG@16
PS2, Line 16: users can just raise the
: new flag, and ideally we will keep the semantics of "I want to be able
: to back up to this point" in future releases, minimizing user-end
: configuration changes.
> I feel like I never really understood this argument, but maybe that's becau
The primary difference is that a new flag doesn't need to be stable. Additionally the new flag implies some "functional" capability, that may change semantics in the future.
With a new flag we don't need to ask users to change the value of a stable flag for an unstable feature.
It gives us the flexibility to re-purpose the flag in the future while maintaining the same functional promise or drop it all together when a new mechanism for anchoring is introduced.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 04 Feb 2019 21:00:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12327/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/3//COMMIT_MSG@9
PS3, Line 9: This patch implements a new flag,
: --backup_max_age_sec, separate from --tablet_history_max_age_sec. Actual
: retention will be maximum of these two flags
What if the value set for the new value is still not enough?
I don't have much context here, but maybe an alternative approach would be: instead GetTabletAncientHistoryMark() returning an earliest of timestamps among all scan operations at snapshot and the timestamp calculated from now for --tablet_history_max_age_sec setting?
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 19:40:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG@16
PS2, Line 16: users can just raise the
: new flag, and ideally we will keep the semantics of "I want to be able
: to back up to this point" in future releases, minimizing user-end
: configuration changes.
> I feel like I never really understood this argument, but maybe that's becau
The use case I had in mind: user has already modified --tablet_history_max_age_sec, we introduce a new flag that is set to a high value to support backups. Use can use backups out of the box. I was imagining that we would set this flag to maybe 1 week by default, but I don't think we should consider doing that before backup & restore are GA.
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/integration-tests/tablet_history_gc-itest.cc
File src/kudu/integration-tests/tablet_history_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/integration-tests/tablet_history_gc-itest.cc@119
PS2, Line 119: FLAGS_backup_max_age_sec = 0;
same here, maybe set to 0 in the constructor
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet_history_gc-test.cc
File src/kudu/tablet/tablet_history_gc-test.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet_history_gc-test.cc@158
PS2, Line 158: FLAGS_backup_max_age_sec = 1;
Consider setting this to 0 in the constructor for TabletHistoryGcTest?
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Mon, 04 Feb 2019 20:54:25 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 4:
> > At scan time, the AHM for admission will be `now - new flag` _for backup scans_
>
> Is there such a thing as a "backup scan"? What criteria defines a scan as a backup scan?
Not today, no.
You could argue that the presence of a second timestamp indicates a backup scan (by virtue of using diff scan functionality). But that's insufficient: as per the argument I laid out earlier, the AHM semantics of full backups need to change as well.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 21:15:04 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Adar Dembo, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12327
to look at the new patch set (#3).
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
KUDU-2677 Implement new gflag for backup history retention
This patch implements a new flag,
--backup_max_age_sec, separate from --tablet_history_max_age_sec. Actual
retention will be maximum of these two flags. The purpose of this is to
smooth the user experience as backup evolves: in the absence of this
flag, for the initial implementation of backup, users will need to raise
--tablet_history_max_age_sec, and then likely lower it again when a
future release adds more backup capabilities or configuration behind the
scenes. With the new --backup_max_age_sec flag, users can just raise the
new flag, and ideally we will keep the semantics of "I want to be able
to back up to this point" in future releases, minimizing user-end
configuration changes.
Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
---
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tools/ksck_remote-test.cc
5 files changed, 96 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/12327/3
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG@16
PS2, Line 16: users can just raise the
: new flag, and ideally we will keep the semantics of "I want to be able
: to back up to this point" in future releases, minimizing user-end
: configuration changes.
I feel like I never really understood this argument, but maybe that's because I'm missing some important detail. Could you flesh out this example some more, perhaps leveraging the future "anchor point" or "snapshot" functionality that's been discussed? Why wouldn't we have the same issue with --backup_max_age_sec as we do with --tablet_history_max_age_sec when we add those capabilities?
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet.cc@150
PS2, Line 150: TAG_FLAG(backup_max_age_sec, experimental);
Probably shouldn't be experimental if anyone who wants to do an incremental backup needs to tweak this.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Sat, 02 Feb 2019 20:16:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG@16
PS2, Line 16: users can just raise the
: new flag, and ideally we will keep the semantics of "I want to be able
: to back up to this point" in future releases, minimizing user-end
: configuration changes.
> The primary difference is that a new flag doesn't need to be stable. Additi
Are those extra bits of color good enough for you Adar? I think their main points are captured in my current commit message.
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/integration-tests/tablet_history_gc-itest.cc
File src/kudu/integration-tests/tablet_history_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/integration-tests/tablet_history_gc-itest.cc@119
PS2, Line 119: FLAGS_backup_max_age_sec = 0;
> same here, maybe set to 0 in the constructor
Done
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet.cc@150
PS2, Line 150: TAG_FLAG(backup_max_age_sec, experimental);
> Probably shouldn't be experimental if anyone who wants to do an incremental
Whoops, I meant evolving.
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet_history_gc-test.cc
File src/kudu/tablet/tablet_history_gc-test.cc:
http://gerrit.cloudera.org:8080/#/c/12327/2/src/kudu/tablet/tablet_history_gc-test.cc@158
PS2, Line 158: FLAGS_backup_max_age_sec = 1;
> Consider setting this to 0 in the constructor for TabletHistoryGcTest?
Done
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 04 Feb 2019 21:09:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 21:44:07 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/2//COMMIT_MSG@16
PS2, Line 16: users can just raise the
: new flag, and ideally we will keep the semantics of "I want to be able
: to back up to this point" in future releases, minimizing user-end
: configuration changes.
> Are those extra bits of color good enough for you Adar? I think their main
We talked about this over Slack and I think I've come around, however not for the arguments raised here by Mike and Grant. Here's my attempt at summarizing the proposal:
The fundamental motivation driving this patch is that it's not safe for us to change the default value of the old --tablet_history_max_age_sec flag. Not because it's a stable flag (it's not), but because of the ramifications for scanning immediately following the change:
1. If we increase the flag's default value, there's now a period of time for which scans are admitted but there's no historical data.
2. If we decrease the flag's default value, there may be scans that were previously admitted that will now fail because they're no longer within the AHM.
Importantly, _any_ user is exposed to these failure modes, _even those that have never changed the value of the old flag_.
So, this change must only affect the behavior of _backup scans_. The contract is simple: it's a new feature, so only folks using the feature will be affected, and by using it they're implicitly opting into the new behavior.
The change:
1. Add a new evolving flag whose default value is much higher than the old (evolving) flag's default value.
2. At compaction time, the AHM for compaction will now be calculated via the formula `now - max(new flag, old flag)`.
3. At scan time, the AHM for admission will be `now - new flag` _for backup scans_, but still `now - old flag` for regular scans.
4. In the future, if we were to replace the new flag with something like persistent anchors, regular scans would behave as they did before #1 (even though a bunch of old history may have been pruned following the upgrade), and it'd be our responsibility to ensure that backup scans would continue to work as they did in #3.
The compaction change affects _everyone_, but by not using the same formula for regular scans as for compactions, we can retain existing expectations for regular scans while changing expectations only for backup scans. That is, we avoid the pitfalls described in the "changing the default of the old flag" section above _for regular scans_.
Importantly, "backup scans" include not just the diff scans used for incremental backups, but all backup-driven scans. Why? Because scans for full backups will be full tablet scans that all use the same HT timestamp; given how a parallel execution engine will schedule these scans, it's quite likely that by the time the last scan runs, the previously chosen HT timestamp will lie well beyond the default 15 minute AHM.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 05 Feb 2019 00:43:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has abandoned this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Abandoned
After more thought and discussion, it seems people are either indifferent to a simpler approach of just raising the max age flag and not introducing a new flag, or they are weakly in favor of a new flag but OK with just raising the max age.
I think we should just raise the existing max age flag. It's simple, the potential pain from it is temporary, and I don't expect really any users will feel it. Users heavily relying on scanning tablet history in a particular way ought to have changed the flag themselves, and so will not be affected by changing the default.
I'll post a new patch that just raises the default value of --tablet_history_max_age_sec.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 4: Code-Review+1
This patch is what I originally envisioned for this change. I can see both sides of the argument: changing a configuration flag for most people is not a big deal, on the other hand people mostly don't want to have to worry about changing configuration flags. Currently Kudu doesn't have many tunables and we try to keep it that way, thus this approach.
I agree that there exists an issue with people being able to scan possibly-invalid history after changing any of these flags, but that's a temporary problem and something that exists already so is orthogonal to this patch.
Overall this patch looks good to me but if others think it's confusing and want to simply increase the value of the main AHM flag then I don't think that is a particularly bad solution either.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 23:33:57 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Grant Henke,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12327
to look at the new patch set (#2).
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
KUDU-2677 Implement new gflag for backup history retention
This patch implements a new flag,
--backup_max_age_sec, separate from --tablet_history_max_age_sec. Actual
retention will be maximum of these two flags. The purpose of this is to
smooth the user experience as backup evolves: in the absence of this
flag, for the initial implementation of backup, users will need to raise
--tablet_history_max_age_sec, and then likely lower it again when a
future release adds more backup capabilities or configuration behind the
scenes. With the new --backup_max_age_sec flag, users can just raise the
new flag, and ideally we will keep the semantics of "I want to be able
to back up to this point" in future releases, minimizing user-end
configuration changes.
Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
---
M src/kudu/integration-tests/tablet_history_gc-itest.cc
M src/kudu/tablet/mt-tablet-test.cc
M src/kudu/tablet/tablet.cc
M src/kudu/tablet/tablet_history_gc-test.cc
M src/kudu/tools/ksck_remote-test.cc
5 files changed, 97 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/27/12327/2
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 4:
> At scan time, the AHM for admission will be `now - new flag` _for backup scans_
Is there such a thing as a "backup scan"? What criteria defines a scan as a backup scan?
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Mon, 06 May 2019 20:31:08 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2677 Implement new gflag for backup history retention
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/12327 )
Change subject: KUDU-2677 Implement new gflag for backup history retention
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12327/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12327/3//COMMIT_MSG@9
PS3, Line 9: This patch implements a new flag,
: --backup_max_age_sec, separate from --tablet_history_max_age_sec. Actual
: retention will be maximum of these two flags
> What if the value set for the new value is still not enough?
We don't currently attempt to pre-open all scanners when starting a large scan (think a Spark job that must scan all partitions in a huge table) so that won't help. The issue is not that there is a danger of corrupting already-opened scanners (we have POSIX-like refcounting semantics when deleting files, so that isn't a problem); the issue is how to ensure opening a new scanner at some relatively old timestamp will be successful when it takes a long time to cycle through scanning all the tablets.
--
To view, visit http://gerrit.cloudera.org:8080/12327
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6c2c001c50f65e9acdfc032e8aa5efdacbd4d9e
Gerrit-Change-Number: 12327
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 06 Feb 2019 19:05:09 +0000
Gerrit-HasComments: Yes