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 <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Tue, 05 Feb 2019 00:43:22 +0000 Gerrit-HasComments: Yes
