[kudu-CR](branch-0.10.x) Change to non-SNAPSHOT version on 0.10.x branch
Todd Lipcon has submitted this change and it was merged. Change subject: Change to non-SNAPSHOT version on 0.10.x branch .. Change to non-SNAPSHOT version on 0.10.x branch Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Reviewed-on: http://gerrit.cloudera.org:8080/4001 Reviewed-by: Mike Percy Tested-by: Todd Lipcon --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 9 files changed, 9 insertions(+), 9 deletions(-) Approvals: Mike Percy: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/4001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Bump version to 1.0.0-SNAPSHOT
Todd Lipcon has submitted this change and it was merged. Change subject: Bump version to 1.0.0-SNAPSHOT .. Bump version to 1.0.0-SNAPSHOT Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c Reviewed-on: http://gerrit.cloudera.org:8080/4000 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 9 files changed, 10 insertions(+), 10 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/4000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR](branch-0.10.x) Change to non-SNAPSHOT version on 0.10.x branch
Todd Lipcon has posted comments on this change. Change subject: Change to non-SNAPSHOT version on 0.10.x branch .. Patch Set 1: Verified+1 Same dist-test/isolate flakiness seen earlier -- To view, visit http://gerrit.cloudera.org:8080/4001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR](branch-0.10.x) Change to non-SNAPSHOT version on 0.10.x branch
Mike Percy has posted comments on this change. Change subject: Change to non-SNAPSHOT version on 0.10.x branch .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] Bump version to 1.0.0-SNAPSHOT
Mike Percy has posted comments on this change. Change subject: Bump version to 1.0.0-SNAPSHOT .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/4000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-HasComments: No
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Todd Lipcon has posted comments on this change. Change subject: KUDU-236 (part 1). Implement tablet history GC .. Patch Set 16: (1 comment) http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: Line 958: is_garbage_collected = true; > Look below at the comment and the DCHECK several lines below. It says it's It's not possible down below, because _after_ the flush starts, you can't get a _new_ reinsert. But you can have existing deltas in the 'REDO' list which are DELETE/REINSERT in the case of an MRS which has had alternating delete/insert _prior_ to the flush starting. -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR](branch-0.10.x) Change to non-SNAPSHOT version on 0.10.x branch
Kudu Jenkins has posted comments on this change. Change subject: Change to non-SNAPSHOT version on 0.10.x branch .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2945/ -- To view, visit http://gerrit.cloudera.org:8080/4001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR](branch-0.10.x) Change to non-SNAPSHOT version on 0.10.x branch
Todd Lipcon has uploaded a new change for review. http://gerrit.cloudera.org:8080/4001 Change subject: Change to non-SNAPSHOT version on 0.10.x branch .. Change to non-SNAPSHOT version on 0.10.x branch Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 9 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/4001/1 -- To view, visit http://gerrit.cloudera.org:8080/4001 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3f8f60893a426388c7c257ae165f9f7e34142d5a Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: branch-0.10.x Gerrit-Owner: Todd Lipcon
[kudu-CR] Bump version to 1.0.0-SNAPSHOT
Kudu Jenkins has posted comments on this change. Change subject: Bump version to 1.0.0-SNAPSHOT .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2944/ -- To view, visit http://gerrit.cloudera.org:8080/4000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] Bump version to 1.0.0-SNAPSHOT
Hello Adar Dembo, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/4000 to review the following change. Change subject: Bump version to 1.0.0-SNAPSHOT .. Bump version to 1.0.0-SNAPSHOT Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c --- M java/interface-annotations/pom.xml M java/kudu-client-tools/pom.xml M java/kudu-client/pom.xml M java/kudu-csd/pom.xml M java/kudu-flume-sink/pom.xml M java/kudu-mapreduce/pom.xml M java/kudu-spark/pom.xml M java/pom.xml M version.txt 9 files changed, 10 insertions(+), 10 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/00/4000/1 -- To view, visit http://gerrit.cloudera.org:8080/4000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I555d01b7704f4bd71559207520b68f64d58cd66c Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Kudu Jenkins has posted comments on this change. Change subject: KUDU-236 (part 1). Implement tablet history GC .. Patch Set 17: Build Started http://104.196.14.100/job/kudu-gerrit/2943/ -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#17). Change subject: KUDU-236 (part 1). Implement tablet history GC .. KUDU-236 (part 1). Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction * Test for ghost rows in multiple RS reinsert case * Test for reupdating missed deltas * Test for partial rowset history GC using alternating rows * Test for major delta compaction against a subset of columns * Test for opening a scanner at TS < AHM and rejecting that scanner at the RPC level Tests coming in a follow-up commit: * Randomized test Missing features coming in follow-up commits: * Background task that schedules and performs undo GC * GC history on delta flush Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 24 files changed, 946 insertions(+), 153 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/17 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Mike Percy has posted comments on this change.
Change subject: KUDU-236 (part 1). Implement tablet history GC
..
Patch Set 16:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:
Line 215:
> some spurious whitespace added
will fix
PS16, Line 387: r
> unused
Done
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:
Line 958: is_garbage_collected = true;
> I don't think this is quite right, because you might hit a REINSERT delta h
Look below at the comment and the DCHECK several lines below. It says it's not
possible.
Line 1038: if (is_garbage_collected) {
> think this is better written: if (!is_garbage_collected) output_row_offset+
Will do.
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/tablet_history_gc-test.cc
File src/kudu/tablet/tablet_history_gc-test.cc:
Line 418: // Delete every even row.
> per elsewhere, I think this test would catch another bug if you do some of
see my comment on the other file
--
To view, visit http://gerrit.cloudera.org:8080/3076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Todd Lipcon has posted comments on this change.
Change subject: KUDU-236 (part 1). Implement tablet history GC
..
Patch Set 16:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction-test.cc
File src/kudu/tablet/compaction-test.cc:
Line 215:
some spurious whitespace added
PS16, Line 387: r
unused
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:
Line 958: is_garbage_collected = true;
I don't think this is quite right, because you might hit a REINSERT delta here
in the case of flushing an MRS.
EG if the MRS has a base row, plus a DELETE plus a REINSERT, and the DELETE is
at a GCable timestamp, we still didn't actually GC the whole row. I think we
need to keep running through the deltas until we see whether the _last_ one is
a GCable DELETE
(would be good to add a test for this case too)
Line 1038: if (is_garbage_collected) {
think this is better written: if (!is_garbage_collected) output_row_offset++;
http://gerrit.cloudera.org:8080/#/c/3076/16/src/kudu/tablet/tablet_history_gc-test.cc
File src/kudu/tablet/tablet_history_gc-test.cc:
Line 418: // Delete every even row.
per elsewhere, I think this test would catch another bug if you do some of the
deletes (and reinsert) while they're still in MRS.
--
To view, visit http://gerrit.cloudera.org:8080/3076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2
Gerrit-PatchSet: 16
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has submitted this change and it was merged. Change subject: Add time/watermark based garbage collection to ResultTracker .. Add time/watermark based garbage collection to ResultTracker This adds time and watermark based garbage collection to the ResultTracker. Regarding time GC, there are two ttl's, a client ttl and a response ttl. After the response ttl has elapsed, we garbage collect responses but the ResultTracker remembers that it doesn't know them, so if the client retries a request older than that it gets a meaningful error back, stating that the request is stale. After the client ttl period without hearing back from a client, we GC the client state entirely, meaning all requests from that client will be treated as new. Regarding watermark GC the algorithm is simple, we trust the client to tell us what's its lowest incomplete sequence number and we GC everything below that. This adds a simple test that makes sure this basically works, and adds a multithreaded test that runs GC at the same time as writes. NOTE: this does not wire the time-based garbage collection process into the server itself -- it's currently only triggered by the included tests. Original patch by David. Some changes by Todd. Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Reviewed-on: http://gerrit.cloudera.org:8080/3628 Reviewed-by: Adar Dembo Tested-by: Todd Lipcon --- M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/retriable_rpc.h D src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/transactions/transaction_driver.cc 11 files changed, 797 insertions(+), 428 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Todd Lipcon: Verified -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 19 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Start a background thread to run ResultTracker GC
Todd Lipcon has submitted this change and it was merged. Change subject: Start a background thread to run ResultTracker GC .. Start a background thread to run ResultTracker GC Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Reviewed-on: http://gerrit.cloudera.org:8080/3961 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/service_if.cc M src/kudu/server/server_base.cc 5 files changed, 54 insertions(+), 25 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 18: Verified+1 the test failure was some weird dist-test flakiness that I'm looking into separately. Overriding since this got a bunch of prior jenkins +1s with only surface-level changes since then -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: rewrite parser logic
Todd Lipcon has submitted this change and it was merged. Change subject: tool: rewrite parser logic .. tool: rewrite parser logic While leaf and non-leaf actions share some common properties, there is much they don't share. Rather than shoehorn both into the same Action paradigm, I think it makes more sense to consider them separately. This patch splits Action into either Mode (non-leaf node) or Action (leaf node). The common properties are now found in the Label struct. Additionally, each kind of node is now structured as a class with proper encapsulation, builders, and other goodies. Overall this simplifies the command line parsing logic, hides more internal details, and reduces the boilerplate needed to add a mode or an action. There's no change to the tool's interface with the outside world. Change-Id: I794fc527525a57283f0165e262283adf14160def Reviewed-on: http://gerrit.cloudera.org:8080/3996 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 5 files changed, 333 insertions(+), 200 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 17: (1 comment) http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: Line 348: completion_record->last_updated = MonoTime::Now(); > Yes, but what of the inverse: if MustHandleRpc==false for all ongoing RPCs, yea, but the record was still "touched" in a sense, so it's still good to bump its expiration time I think. Maybe "last_touched" would be a better name -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: rewrite parser logic
Todd Lipcon has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Adar Dembo has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 18: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: Line 348: int64_t seq_no = request_id.seq_no(); > not convinced of this one -- if we moved it inside the loop, it might get u Yes, but what of the inverse: if MustHandleRpc==false for all ongoing RPCs, we're changing last_updated for a CR that didn't actually change. -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: rewrite parser logic
Adar Dembo has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 3: > looks good but the patch it's based on seems to have exploded. Yeah, I rebased to switch the order around. And I thought _that_ patch was the non-controversial one. Guess it needs more work. -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: rewrite parser logic
Kudu Jenkins has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2942/ -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: rewrite parser logic
Todd Lipcon has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: rewrite parser logic
Todd Lipcon has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 2: looks good but the patch it's based on seems to have exploded. -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] integration tests: end-to-end master permanent failure test
Todd Lipcon has submitted this change and it was merged. Change subject: integration_tests: end-to-end master permanent failure test .. integration_tests: end-to-end master permanent failure test This commit defines a workflow for handling master permanent failures, and includes an integration test to execute it. In production, the workflow will be run manually by an admin, or via a script. As part of this change, I added support for optional parameters to the new CLI tool. It was straight-forward to reuse gflags for this. Still to come: CLI help for positional (i.e. required) parameters. Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Reviewed-on: http://gerrit.cloudera.org:8080/3969 Tested-by: Kudu Jenkins Reviewed-by: Todd Lipcon --- M src/kudu/client/client-test-util.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 218 insertions(+), 31 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] integration tests: end-to-end master permanent failure test
Todd Lipcon has posted comments on this change. Change subject: integration_tests: end-to-end master permanent failure test .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 18: kicked off dist-test runs with 1000 of each of the integration tests here: http://dist-test.cloudera.org/job?job_id=todd.1471323037.16714 http://dist-test.cloudera.org/job?job_id=todd.1471323329.17166 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3628 to look at the new patch set (#18). Change subject: Add time/watermark based garbage collection to ResultTracker .. Add time/watermark based garbage collection to ResultTracker This adds time and watermark based garbage collection to the ResultTracker. Regarding time GC, there are two ttl's, a client ttl and a response ttl. After the response ttl has elapsed, we garbage collect responses but the ResultTracker remembers that it doesn't know them, so if the client retries a request older than that it gets a meaningful error back, stating that the request is stale. After the client ttl period without hearing back from a client, we GC the client state entirely, meaning all requests from that client will be treated as new. Regarding watermark GC the algorithm is simple, we trust the client to tell us what's its lowest incomplete sequence number and we GC everything below that. This adds a simple test that makes sure this basically works, and adds a multithreaded test that runs GC at the same time as writes. NOTE: this does not wire the time-based garbage collection process into the server itself -- it's currently only triggered by the included tests. Original patch by David. Some changes by Todd. Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 --- M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/retriable_rpc.h D src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/transactions/transaction_driver.cc 11 files changed, 797 insertions(+), 428 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/3628/18 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Kudu Jenkins has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 18: Build Started http://104.196.14.100/job/kudu-gerrit/2941/ -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 18 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 17: (6 comments) http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/CMakeLists.txt File src/kudu/rpc/CMakeLists.txt: Line 121: ADD_KUDU_TEST(exactly_once_rpc-test) > Can you resort the tests? Done http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: PS17, Line 152: completion_record->state = RpcState::IN_PROGRESS; : completion_record->driver_attempt_no = request_id.attempt_no(); > Can we add these to the CR constructor? Done Line 232: completion_record->last_updated = MonoTime::Now(); > Technically we haven't mutated the CR, so why update its timestamp? yea, not sure, will remove. Line 348: completion_record->last_updated = MonoTime::Now(); > Likewise, shouldn't we push this down into where we actually update the CR? not convinced of this one -- if we moved it inside the loop, it might get updated multiple times, and no sense in wasting the extra clock calls. http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: PS17, Line 1247: // We only replay committed requests so the result of tracking this request can be: : // NEW - This is a previously untracked request, or we changed the driver -> store the result : // COMPLETED - We've bootstrapped this tablet twice, and previously stored the result -> do : // nothing. > Update this comment. Done http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/tablet/transactions/transaction_driver.cc File src/kudu/tablet/transactions/transaction_driver.cc: PS17, Line 227: VLOG(1) << state()->result_tracker() << " Follower Rpc was not NEW or IN_PROGRESS: " : << rpc_state << " OpId: " << state()->op_id().ShortDebugString() : << " RequestId: " << state()->request_id().ShortDebugString(); > Update this too? Should have been "not NEW or COMPLETED", I think. done. also changed to VLOG(2) since this is quite internal -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 17: Yea, gerrit's poor at dealing with renames. git show -M does a better job. For your convenience, here's the git show -M for the test: https://gist.github.com/toddlipcon/c150de547f7a6b8462eeaeb568efdae9 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3952 to look at the new patch set (#8). Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode Implemented AUTO_FLUSH_BACKGROUND for the Kudu C++ client library. KuduSession objects use RPC messenger's thread pool to schedule background flush tasks. In AUTO_FLUSH_BACKGROUND mode, the KuduSession::Apply() method blocks if total amount of data for operations not pushed to server reaches the buffer size limit. The limit on the buffer size can be set by the KuduSession::SetMutationBufferSpace() method or via the --client_buffer_bytes_limit command-line flag. The background flusher checks whether at least one of the following two conditions is satisfied to determine whether it's time to flush the accumulated write operations: * over-the-waterline criterion: check whether the total size of the freshly submitted (i.e. not-scheduled-for-flush) write operations is over the threshold. The threshold can be set as a percentage of the total buffer size using the --client_buffer_flush_waterline_pct command-line flag. * maximum wait time criterion: check whether the previous flush happened more than the maximum wait time ago. The maximum wait time can be specified in milliseconds using the --client_buffer_flush_timeout_ms command-line flag. This change also addresses the following JIRA issue: KUDU-1376 KuduSession::SetMutationBufferSpace is not defined Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 --- M python/kudu/tests/test_client.py M src/kudu/client/batcher.cc M src/kudu/client/batcher.h M src/kudu/client/client-internal.h M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/session-internal.cc M src/kudu/client/session-internal.h M src/kudu/client/write_op.cc M src/kudu/client/write_op.h M src/kudu/util/monotime.cc M src/kudu/util/monotime.h 13 files changed, 1,209 insertions(+), 154 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/3952/8 -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Kudu Jenkins has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 8: Build Started http://104.196.14.100/job/kudu-gerrit/2940/ -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [util/monotime] added handy operators for MonoTime
Kudu Jenkins has posted comments on this change. Change subject: [util/monotime] added handy operators for MonoTime .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2939/ -- To view, visit http://gerrit.cloudera.org:8080/3999 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia8a336bc79461adebfb2d5c136d71f90efcd36db Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] [util/monotime] added handy operators for MonoTime
Alexey Serbin has uploaded a new change for review. http://gerrit.cloudera.org:8080/3999 Change subject: [util/monotime] added handy operators for MonoTime .. [util/monotime] added handy operators for MonoTime Change-Id: Ia8a336bc79461adebfb2d5c136d71f90efcd36db --- M src/kudu/util/monotime-test.cc M src/kudu/util/monotime.cc M src/kudu/util/monotime.h 3 files changed, 312 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/3999/1 -- To view, visit http://gerrit.cloudera.org:8080/3999 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ia8a336bc79461adebfb2d5c136d71f90efcd36db Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#16). Change subject: KUDU-236 (part 1). Implement tablet history GC .. KUDU-236 (part 1). Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction * Test for ghost rows in multiple RS reinsert case * Test for reupdating missed deltas * Test for partial rowset history GC using alternating rows * Test for major delta compaction against a subset of columns * Test for opening a scanner at TS < AHM and rejecting that scanner at the RPC level Tests coming in a follow-up commit: * Randomized test Missing features coming in follow-up commits: * Background task that schedules and performs undo GC * GC history on delta flush Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 24 files changed, 952 insertions(+), 153 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/16 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Kudu Jenkins has posted comments on this change. Change subject: KUDU-236 (part 1). Implement tablet history GC .. Patch Set 16: Build Started http://104.196.14.100/job/kudu-gerrit/2938/ -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Mike Percy has posted comments on this change. Change subject: KUDU-236 (part 1). Implement tablet history GC .. Patch Set 15: > > > Could you also add a GCing thread to mt-tablet-test? It's a > good > > > way to test interactions between concurrent tablet operations. > > > > There's no new dedicated GC task yet. The GCing happens as part > of > > flush and compaction. That new MM task is coming later. > > mt-tablet-test doesn't invoke MM tasks though; it just calls public > Tablet methods. I only skimmed this patch but I think it adds > everything needed to extend the test. The way we would do it is simply set the tablet_history_max_age to some low value like 1 or 2 in the test, I guess. Everything else would be automatic, assuming it's already using HybridTime. That's something I can do. -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Support downgrade to version that has LocalConsensus
Mike Percy has posted comments on this change. Change subject: Support downgrade to version that has LocalConsensus .. Patch Set 2: > Can you please give this a manual test as part of voting on 0.10 RC? Sure, no problem. -- To view, visit http://gerrit.cloudera.org:8080/3985 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6d324b4f3ce5906bfc9a4df49dd60ca0cb919820 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Adar Dembo has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 17: (6 comments) It's pretty tough to review exactly_once_rpc-test.cc since gerrit is showing it as all new. Any chance you can break out the rename of the test into a separate patch? Alternatively, you can describe what's actually changed in the test and I can focus on that. http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/CMakeLists.txt File src/kudu/rpc/CMakeLists.txt: Line 121: ADD_KUDU_TEST(exactly_once_rpc-test) Can you resort the tests? http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/rpc/result_tracker.cc File src/kudu/rpc/result_tracker.cc: PS17, Line 152: completion_record->state = RpcState::IN_PROGRESS; : completion_record->driver_attempt_no = request_id.attempt_no(); Can we add these to the CR constructor? Line 232: completion_record->last_updated = MonoTime::Now(); Technically we haven't mutated the CR, so why update its timestamp? Line 348: completion_record->last_updated = MonoTime::Now(); Likewise, shouldn't we push this down into where we actually update the CR? I think that's L365, though I'm not sure. http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: PS17, Line 1247: // We only replay committed requests so the result of tracking this request can be: : // NEW - This is a previously untracked request, or we changed the driver -> store the result : // COMPLETED - We've bootstrapped this tablet twice, and previously stored the result -> do : // nothing. Update this comment. http://gerrit.cloudera.org:8080/#/c/3628/17/src/kudu/tablet/transactions/transaction_driver.cc File src/kudu/tablet/transactions/transaction_driver.cc: PS17, Line 227: VLOG(1) << state()->result_tracker() << " Follower Rpc was not NEW or IN_PROGRESS: " : << rpc_state << " OpId: " << state()->op_id().ShortDebugString() : << " RequestId: " << state()->request_id().ShortDebugString(); Update this too? Should have been "not NEW or COMPLETED", I think. -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: rewrite parser logic
Hello Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3996 to look at the new patch set (#2). Change subject: tool: rewrite parser logic .. tool: rewrite parser logic While leaf and non-leaf actions share some common properties, there is much they don't share. Rather than shoehorn both into the same Action paradigm, I think it makes more sense to consider them separately. This patch splits Action into either Mode (non-leaf node) or Action (leaf node). The common properties are now found in the Label struct. Additionally, each kind of node is now structured as a class with proper encapsulation, builders, and other goodies. Overall this simplifies the command line parsing logic, hides more internal details, and reduces the boilerplate needed to add a mode or an action. There's no change to the tool's interface with the outside world. Change-Id: I794fc527525a57283f0165e262283adf14160def --- M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 5 files changed, 333 insertions(+), 200 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3996/2 -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] catalog manager: avoid more races between Init() and GetTabletPeer()
Hello Todd Lipcon, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/3997 to review the following change. Change subject: catalog_manager: avoid more races between Init() and GetTabletPeer() .. catalog_manager: avoid more races between Init() and GetTabletPeer() Commit 2525ad0 took a stab at this, but apparently it wasn't enough due to the tablet_id() call. So here's another attempt, where sys_catalog_ is only set when it is fully formed (i.e. when it has a functional TabletPeer). Below I've included test output when the race hits. master_replication-itest: /home/jenkins-slave/workspace/kudu-3/src/kudu/gutil/ref_counted.h:273: T *scoped_refptr::operator->() const [T = kudu::tablet::TabletPeer]: Assertion `ptr_ != __null' failed. *** Aborted at 1471309445 (unix time) try "date -d @1471309445" if you are using GNU date *** PC: @ 0x7f330225dcc9 gsignal *** SIGABRT (@0x3e86e90) received by PID 28304 (TID 0x7f32f06eb700) from PID 28304; stack trace: *** @ 0x42e687 __tsan::CallUserSignalHandler() at /home/jenkins-slave/workspace/kudu-3/thirdparty/llvm-3.8.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1962 @ 0x42f4d3 rtl_sigaction() at /home/jenkins-slave/workspace/kudu-3/thirdparty/llvm-3.8.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:2039 @ 0x7f33090a4340 (unknown) at ??:0 @ 0x7f330225dcc9 gsignal at ??:0 @ 0x7f33022610d8 abort at ??:0 @ 0x7f3302256b86 (unknown) at ??:0 @ 0x7f3302256c32 __assert_fail at ??:0 @ 0x7f330ca13130 scoped_refptr<>::operator->() at ??:0 @ 0x7f330ca1a952 kudu::master::SysCatalogTable::tablet_id() at ??:0 @ 0x7f330ca0b136 kudu::master::CatalogManager::GetTabletPeer() at ??:0 @ 0x7f330c69214d kudu::tserver::(anonymous namespace)::LookupTabletPeerOrRespond<>() at ??:0 @ 0x7f330c691bab kudu::tserver::ConsensusServiceImpl::RequestConsensusVote() at ??:0 @ 0x7f3307c9fca5 kudu::consensus::ConsensusServiceIf::ConsensusServiceIf()::$_1::operator()() at ??:0 @ 0x7f3307c9fabf std::_Function_handler<>::_M_invoke() at ??:0 @ 0x7f3306bd7219 std::function<>::operator()() at ??:0 @ 0x7f3306bd6c8e kudu::rpc::GeneratedServiceIf::Handle() at ??:0 @ 0x7f3306bd8b3e kudu::rpc::ServicePool::RunThread() at ??:0 @ 0x7f3306bdaa27 boost::_mfi::mf0<>::operator()() at ??:0 @ 0x7f3306bda98b boost::_bi::list1<>::operator()<>() at ??:0 @ 0x7f3306bda934 boost::_bi::bind_t<>::operator()() at ??:0 @ 0x7f3306bda75a boost::detail::function::void_function_obj_invoker0<>::invoke() at ??:0 @ 0x7f3306b758b2 boost::function0<>::operator()() at ??:0 @ 0x7f3304962630 kudu::Thread::SuperviseThread() at ??:0 Change-Id: I43fdc6499cb84d2053bed08b689fe5a08a6761d6 --- M src/kudu/master/catalog_manager.cc 1 file changed, 9 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/3997/1 -- To view, visit http://gerrit.cloudera.org:8080/3997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I43fdc6499cb84d2053bed08b689fe5a08a6761d6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Todd Lipcon
[kudu-CR] integration tests: end-to-end master permanent failure test
Hello Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3969 to look at the new patch set (#4). Change subject: integration_tests: end-to-end master permanent failure test .. integration_tests: end-to-end master permanent failure test This commit defines a workflow for handling master permanent failures, and includes an integration test to execute it. In production, the workflow will be run manually by an admin, or via a script. As part of this change, I added support for optional parameters to the new CLI tool. It was straight-forward to reuse gflags for this. Still to come: CLI help for positional (i.e. required) parameters. Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 --- M src/kudu/client/client-test-util.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 218 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/3969/4 -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: rewrite parser logic
Kudu Jenkins has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 2: Build Started http://104.196.14.100/job/kudu-gerrit/2936/ -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] tool: rewrite parser logic
Adar Dembo has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3996/1/src/kudu/tools/tool_action.h File src/kudu/tools/tool_action.h: PS1, Line 66: // Invokes an action, passing in the complete action chain and all remaining : // command line arguments. The arguments are passed by value so that the : // function can modify them if need be. > what's this comment attached to? Oops, meant to remove that. PS1, Line 94: // After Build() is called, submodes_ and actions_ are empty. > hrm, isn't it better to just say 'May be only called once'? OK. -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] catalog manager: avoid more races between Init() and GetTabletPeer()
Kudu Jenkins has posted comments on this change. Change subject: catalog_manager: avoid more races between Init() and GetTabletPeer() .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2937/ -- To view, visit http://gerrit.cloudera.org:8080/3997 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I43fdc6499cb84d2053bed08b689fe5a08a6761d6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] integration tests: end-to-end master permanent failure test
Kudu Jenkins has posted comments on this change. Change subject: integration_tests: end-to-end master permanent failure test .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2935/ -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1474: single to multi-master deployment migration
Adar Dembo has submitted this change and it was merged. Change subject: KUDU-1474: single to multi-master deployment migration .. KUDU-1474: single to multi-master deployment migration This patch introduces the machinery needed to migrate from a single-node master deployment to multi-master: 1. Inclusion of the tablet copy service in the master. This was easy; the master module already depends on 'tserver', so registering the service is just a few lines of code. 2. The new "kudu" command line tool, whose operations are used in migration. It'll also serve as the basis for KUDU-619. 3. An end-to-end integration test to showcase the migration. A couple notes about the new tool. I began with a gflags-based approach, but found it unwieldy after a while, because it's tough to provide a good help experience, and the lack of positional argument support makes for long command lines. I briefly looked at boost but didn't want to reintroduce a library dependency, so I ended up writing my own (simple) parser. It maps command line arguments to "actions" and allows for arbitrary levels of nesting, so you can do things like "kudu tablet copy blah" as well as "kudu fs dump". At each level, if insufficient arguments are provided, an informative help message is printed though there's room for improvement here. Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Reviewed-on: http://gerrit.cloudera.org:8080/3880 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M build-support/dist_test.py M src/kudu/gutil/strings/join.h M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/master_migration-itest.cc M src/kudu/master/master.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tools/CMakeLists.txt A src/kudu/tools/tool_action.cc A src/kudu/tools/tool_action.h A src/kudu/tools/tool_action_fs.cc A src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_main.cc 15 files changed, 926 insertions(+), 11 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] fs: allow format with user-specified uuid
Adar Dembo has submitted this change and it was merged. Change subject: fs: allow format with user-specified uuid .. fs: allow format with user-specified uuid The CLI tool is going to use this for the "handling permanent failure" workflow. Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Reviewed-on: http://gerrit.cloudera.org:8080/3968 Reviewed-by: Todd Lipcon Tested-by: Adar Dembo --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/util/CMakeLists.txt A src/kudu/util/oid_generator-test.cc M src/kudu/util/oid_generator.cc M src/kudu/util/oid_generator.h 7 files changed, 139 insertions(+), 14 deletions(-) Approvals: Adar Dembo: Verified Todd Lipcon: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Todd Lipcon
[kudu-CR] fs: allow format with user-specified uuid
Adar Dembo has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 5: Verified+1 The test failure was unrelated, and I've got a patch ready to go for it. -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3076 to look at the new patch set (#15). Change subject: KUDU-236 (part 1). Implement tablet history GC .. KUDU-236 (part 1). Implement tablet history GC This patch defines the concept of an "ancient history mark" (AHM) and a background job that removes history from disk that represents changes occurring prior to the ancient history mark. There is also a mechanism to reject requests for scans if they attempt to open a snapshot scan at a timestamp prior to the AHM. Included tests: * Test for Redo GC via major delta compaction * Test for history GC via tablet flush * Test for Undo GC via merge compaction * Test for entire-row GC via merge compaction * Test for ghost rows in multiple RS reinsert case * Test for reupdating missed deltas * Test for partial rowset history GC using alternating rows * Test for major delta compaction against a subset of columns * Test for opening a scanner at TS < AHM and rejecting that scanner at the RPC level Tests coming in a follow-up commit: * Randomized test Missing features coming in follow-up commits: * Background task that schedules and performs undo GC * GC history on delta flush Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 --- M src/kudu/common/timestamp.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/tablet_history_gc-itest.cc M src/kudu/server/clock.h M src/kudu/server/hybrid_clock.cc M src/kudu/server/hybrid_clock.h M src/kudu/tablet/CMakeLists.txt M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/mutation.h M src/kudu/tablet/rowset_metadata.h M src/kudu/tablet/tablet-harness.h M src/kudu/tablet/tablet-test-base.h M src/kudu/tablet/tablet-test-util.h M src/kudu/tablet/tablet-test.cc M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h A src/kudu/tablet/tablet_history_gc-test.cc M src/kudu/tserver/tablet_service.cc 24 files changed, 951 insertions(+), 152 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/3076/15 -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Kudu Jenkins has posted comments on this change. Change subject: KUDU-236 (part 1). Implement tablet history GC .. Patch Set 15: Build Started http://104.196.14.100/job/kudu-gerrit/2934/ -- To view, visit http://gerrit.cloudera.org:8080/3076 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-236 (part 1). Implement tablet history GC
Mike Percy has posted comments on this change.
Change subject: KUDU-236 (part 1). Implement tablet history GC
..
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3076/14/src/kudu/tablet/compaction.cc
File src/kudu/tablet/compaction.cc:
Line 791: vector* gced_input_rows) {
> I'm not really convinced of this vector here -- it's a bit expensive memory
Ah, good point. Thanks for the help on this code path. Done.
--
To view, visit http://gerrit.cloudera.org:8080/3076
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If9833a863f118eb82be80ea56204d0d9141611c2
Gerrit-PatchSet: 14
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] tool: rewrite parser logic
Todd Lipcon has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3996/1/src/kudu/tools/tool_action.h File src/kudu/tools/tool_action.h: PS1, Line 66: // Invokes an action, passing in the complete action chain and all remaining : // command line arguments. The arguments are passed by value so that the : // function can modify them if need be. what's this comment attached to? PS1, Line 94: // After Build() is called, submodes_ and actions_ are empty. hrm, isn't it better to just say 'May be only called once'? -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] integration tests: end-to-end master permanent failure test
Todd Lipcon has posted comments on this change. Change subject: integration_tests: end-to-end master permanent failure test .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1474: single to multi-master deployment migration
Todd Lipcon has posted comments on this change. Change subject: KUDU-1474: single to multi-master deployment migration .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] fs: allow format with user-specified uuid
Todd Lipcon has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] fs: allow format with user-specified uuid
Adar Dembo has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/3968/4/src/kudu/util/oid_generator-test.cc File src/kudu/util/oid_generator-test.cc: PS4, Line 43: cd > might want to throw in some capitals. Done -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] tool: rewrite parser logic
Kudu Jenkins has posted comments on this change. Change subject: tool: rewrite parser logic .. Patch Set 1: Build Started http://104.196.14.100/job/kudu-gerrit/2933/ -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: No
[kudu-CR] integration tests: end-to-end master permanent failure test
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3969 to look at the new patch set (#3). Change subject: integration_tests: end-to-end master permanent failure test .. integration_tests: end-to-end master permanent failure test This commit defines a workflow for handling master permanent failures, and includes an integration test to execute it. In production, the workflow will be run manually by an admin, or via a script. As part of this change, I added support for optional parameters to the new CLI tool. It was straight-forward to reuse gflags for this. Still to come: CLI help for positional (i.e. required) parameters. Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 --- M src/kudu/client/client-test-util.cc M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_migration-itest.cc M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 8 files changed, 218 insertions(+), 31 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/3969/3 -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] integration tests: end-to-end master permanent failure test
Kudu Jenkins has posted comments on this change. Change subject: integration_tests: end-to-end master permanent failure test .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2932/ -- To view, visit http://gerrit.cloudera.org:8080/3969 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1474: single to multi-master deployment migration
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3880 to look at the new patch set (#7). Change subject: KUDU-1474: single to multi-master deployment migration .. KUDU-1474: single to multi-master deployment migration This patch introduces the machinery needed to migrate from a single-node master deployment to multi-master: 1. Inclusion of the tablet copy service in the master. This was easy; the master module already depends on 'tserver', so registering the service is just a few lines of code. 2. The new "kudu" command line tool, whose operations are used in migration. It'll also serve as the basis for KUDU-619. 3. An end-to-end integration test to showcase the migration. A couple notes about the new tool. I began with a gflags-based approach, but found it unwieldy after a while, because it's tough to provide a good help experience, and the lack of positional argument support makes for long command lines. I briefly looked at boost but didn't want to reintroduce a library dependency, so I ended up writing my own (simple) parser. It maps command line arguments to "actions" and allows for arbitrary levels of nesting, so you can do things like "kudu tablet copy blah" as well as "kudu fs dump". At each level, if insufficient arguments are provided, an informative help message is printed though there's room for improvement here. Change-Id: I89c741381ced3731736228cd07fe85106ae72541 --- M build-support/dist_test.py M src/kudu/gutil/strings/join.h M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/external_mini_cluster.cc M src/kudu/integration-tests/external_mini_cluster.h A src/kudu/integration-tests/master_migration-itest.cc M src/kudu/master/master.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h M src/kudu/tools/CMakeLists.txt A src/kudu/tools/tool_action.cc A src/kudu/tools/tool_action.h A src/kudu/tools/tool_action_fs.cc A src/kudu/tools/tool_action_tablet.cc A src/kudu/tools/tool_main.cc 15 files changed, 926 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/80/3880/7 -- To view, visit http://gerrit.cloudera.org:8080/3880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] tool: rewrite parser logic
Adar Dembo has uploaded a new change for review. http://gerrit.cloudera.org:8080/3996 Change subject: tool: rewrite parser logic .. tool: rewrite parser logic While leaf and non-leaf actions share some common properties, there is much they don't share. Rather than shoehorn both into the same Action paradigm, I think it makes more sense to consider them separately. This patch splits Action into either Mode (non-leaf node) or Action (leaf node). The common properties are now found in the Label struct. Additionally, each kind of node is now structured as a class with proper encapsulation, builders, and other goodies. Overall this simplifies the command line parsing logic, hides more internal details, and reduces the boilerplate needed to add a mode or an action. There's no change to the tool's interface with the outside world. Change-Id: I794fc527525a57283f0165e262283adf14160def --- M src/kudu/tools/tool_action.cc M src/kudu/tools/tool_action.h M src/kudu/tools/tool_action_fs.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_main.cc 5 files changed, 340 insertions(+), 203 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/96/3996/1 -- To view, visit http://gerrit.cloudera.org:8080/3996 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I794fc527525a57283f0165e262283adf14160def Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo
[kudu-CR] KUDU-1474: single to multi-master deployment migration
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1474: single to multi-master deployment migration .. Patch Set 7: Build Started http://104.196.14.100/job/kudu-gerrit/2931/ -- To view, visit http://gerrit.cloudera.org:8080/3880 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] KUDU-1474: single to multi-master deployment migration
Adar Dembo has posted comments on this change.
Change subject: KUDU-1474: single to multi-master deployment migration
..
Patch Set 6:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/gutil/strings/join.h
File src/kudu/gutil/strings/join.h:
Line 204: string JoinStrings(const CONTAINER& components,
> some comment doc here would be good. Perhaps a different name as well, like
Done
http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 45: msg += "Action can be one of the following:\n";
> this function only seems useful/sensical if 'sub_actions' is non-empty, but
Yeah, L63 is dead code; there's no invocation of BuildNonLeafActionHelpString()
with an empty chain. Removed it.
http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action.h
File src/kudu/tools/tool_action.h:
Line 98: Status CheckNoMoreArgs(const std::vector& chain, CONTAINER
args) {
> I think this can take the CONTAINER by const ref.
Done
http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
Line 124: RETURN_NOT_OK(env_util::CopyFile(env, cmeta_filename,
backup_filename, opts));
> worth a LOG(INFO) about 'backing up current config to '
Done
http://gerrit.cloudera.org:8080/#/c/3880/6/src/kudu/tools/tool_main.cc
File src/kudu/tools/tool_main.cc:
Line 69: vector sub_actions = cur->sub_actions;
> is this mutated? or can it be const auto&?
Done
Line 130: FLAGS_helpfull ||
> it seems like it would be useful to still allow --helpfull or something to
Alright. I'm not sure how useful the real help is, but I won't block it.
--
To view, visit http://gerrit.cloudera.org:8080/3880
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I89c741381ced3731736228cd07fe85106ae72541
Gerrit-PatchSet: 6
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] fs: allow format with user-specified uuid
Hello Todd Lipcon, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3968 to look at the new patch set (#5). Change subject: fs: allow format with user-specified uuid .. fs: allow format with user-specified uuid The CLI tool is going to use this for the "handling permanent failure" workflow. Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/util/CMakeLists.txt A src/kudu/util/oid_generator-test.cc M src/kudu/util/oid_generator.cc M src/kudu/util/oid_generator.h 7 files changed, 139 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/3968/5 -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] fs: allow format with user-specified uuid
Kudu Jenkins has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2930/ -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] fs: allow format with user-specified uuid
Dan Burkert has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 4: (1 comment) LGTM except for nit http://gerrit.cloudera.org:8080/#/c/3968/4/src/kudu/util/oid_generator-test.cc File src/kudu/util/oid_generator-test.cc: PS4, Line 43: cd might want to throw in some capitals. -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] integration tests: end-to-end master permanent failure test
Todd Lipcon has posted comments on this change.
Change subject: integration_tests: end-to-end master permanent failure test
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/3969/2/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 57: for (const auto& gflag : action.gflags) {
> Pretty neat.
Looks glorious
--
To view, visit http://gerrit.cloudera.org:8080/3969
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Start a background thread to run ResultTracker GC
Adar Dembo has posted comments on this change. Change subject: Start a background thread to run ResultTracker GC .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Another pass on 0.10.0 release notes
Todd Lipcon has posted comments on this change. Change subject: Another pass on 0.10.0 release notes .. Patch Set 3: k, i just committed, we can do one more patch, but want to get an RC out soon. -- To view, visit http://gerrit.cloudera.org:8080/3979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4414bdebb15d976c025dfce5a3f2bda5768bd5a9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Another pass on 0.10.0 release notes
Todd Lipcon has submitted this change and it was merged. Change subject: Another pass on 0.10.0 release notes .. Another pass on 0.10.0 release notes Change-Id: I4414bdebb15d976c025dfce5a3f2bda5768bd5a9 Reviewed-on: http://gerrit.cloudera.org:8080/3979 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo --- M docs/release_notes.adoc 1 file changed, 49 insertions(+), 10 deletions(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4414bdebb15d976c025dfce5a3f2bda5768bd5a9 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] integration tests: end-to-end master permanent failure test
Adar Dembo has posted comments on this change.
Change subject: integration_tests: end-to-end master permanent failure test
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/3969/2/src/kudu/tools/tool_action.cc
File src/kudu/tools/tool_action.cc:
Line 57: for (const auto& gflag : action.gflags) {
> what about using google::DescribeOneFlag for this? It handles nice line-wra
Pretty neat.
I did retain the majority of this code, though, to provide a nicer usage
string. What do you think of this?
adar@adar-ThinkPad-T540p:~/Source/kudu/build/debug$ bin/kudu fs print_uuid
--help
Usage: bin/kudu fs print_uuid [-fs_wal_dir=] [-fs_data_dirs=]
Print the UUID of a Kudu filesystem
-fs_wal_dir (Directory with write-ahead logs. If this is not specified,
the
program will not start. May be the same as fs_data_dirs) type: string
default: ""
-fs_data_dirs (Comma-separated list of directories with data blocks. If
this is not specified, fs_wal_dir will be used as the sole data block
directory.) type: string default: ""
http://gerrit.cloudera.org:8080/#/c/3969/2/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
Line 203: tablet_print_peer_uuids.name = "print_uuids";
> Isn't this more accurately 'replica uuids', since I think it contains all r
Changed to "print_replica_uuids".
Line 204: tablet_print_peer_uuids.description = "Print the uuids from a
tablet's Raft configuration";
> UUIDs of the peers
Done
--
To view, visit http://gerrit.cloudera.org:8080/3969
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibfab2561e066f00fe56eb4f5d6d6ccbbb2dcbed5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Dan Burkert
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-HasComments: Yes
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Kudu Jenkins has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 17: Build Started http://104.196.14.100/job/kudu-gerrit/2929/ -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Start a background thread to run ResultTracker GC
Hello Adar Dembo, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3961 to look at the new patch set (#5). Change subject: Start a background thread to run ResultTracker GC .. Start a background thread to run ResultTracker GC Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c --- M src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/service_if.cc M src/kudu/server/server_base.cc 5 files changed, 54 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/3961/5 -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3628 to look at the new patch set (#17). Change subject: Add time/watermark based garbage collection to ResultTracker .. Add time/watermark based garbage collection to ResultTracker This adds time and watermark based garbage collection to the ResultTracker. Regarding time GC, there are two ttl's, a client ttl and a response ttl. After the response ttl has elapsed, we garbage collect responses but the ResultTracker remembers that it doesn't know them, so if the client retries a request older than that it gets a meaningful error back, stating that the request is stale. After the client ttl period without hearing back from a client, we GC the client state entirely, meaning all requests from that client will be treated as new. Regarding watermark GC the algorithm is simple, we trust the client to tell us what's its lowest incomplete sequence number and we GC everything below that. This adds a simple test that makes sure this basically works, and adds a multithreaded test that runs GC at the same time as writes. NOTE: this does not wire the time-based garbage collection process into the server itself -- it's currently only triggered by the included tests. Original patch by David. Some changes by Todd. Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 --- M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/retriable_rpc.h D src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/transactions/transaction_driver.cc 11 files changed, 786 insertions(+), 423 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/3628/17 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 17 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] fs: allow format with user-specified uuid
Todd Lipcon has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Start a background thread to run ResultTracker GC
Kudu Jenkins has posted comments on this change. Change subject: Start a background thread to run ResultTracker GC .. Patch Set 5: Build Started http://104.196.14.100/job/kudu-gerrit/2928/ -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 15: (8 comments) http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: PS15, Line 84: cncurrently > concurrently + period Done PS15, Line 86: > word missing Done PS15, Line 95: For > Extra word? Something is off in this sentence. does a comma after 'batches' make it read better? http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/exactly_once_rpc-test.cc File src/kudu/rpc/exactly_once_rpc-test.cc: Line 40: unique_ptr request_id(new RequestIdPB()); > So I know this is a little late to the game, but RpcController holding it's yea, it's a little strange. want to file a follow-up JIRA? It's baked throughout a bunch of places here. http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/result_tracker.h File src/kudu/rpc/result_tracker.h: Line 225: // Runs garbage collection on the results this result tracker is caching. > I would change this to 'Runs time-based garbage collection...' to distingui Done PS15, Line 307: MustGcRecordFunc func > Ideally this would be a template param instead of a std::function so that i Done http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/rpc_header.proto File src/kudu/rpc/rpc_header.proto: Line 126: required int64 first_incomplete_seq_no = 3; > All of these fields should be optional, we shouldn't be introducing any mor why? given this struct itself is optional, isn't it safe for its first introduction to have required fields? In this case though I guess I agree that semantically it's fine for it to be missing (it just means no GC). I'm guessing David made it required so that we dont have cases with misbehaving clients which don't set it and thus end up causing the server to exhaust a lot of memory? http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: Line 1253: || state == ResultTracker::RpcState::COMPLETED > || on previous lines Done -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] fs: allow format with user-specified uuid
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3968 to look at the new patch set (#4). Change subject: fs: allow format with user-specified uuid .. fs: allow format with user-specified uuid The CLI tool is going to use this for the "handling permanent failure" workflow. Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/util/CMakeLists.txt A src/kudu/util/oid_generator-test.cc M src/kudu/util/oid_generator.cc M src/kudu/util/oid_generator.h 7 files changed, 136 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/3968/4 -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] fs: allow format with user-specified uuid
Kudu Jenkins has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2927/ -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] fs: allow format with user-specified uuid
Adar Dembo has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/3968/2/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: Line 326: metadata->set_uuid(uuid.get()); > As we discussed on Slack, I'm -1 on this if it breaks the UUID invariant on You get a validation and you get a validation! Everyone gets a validation! -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] fs: allow format with user-specified uuid
Kudu Jenkins has posted comments on this change. Change subject: fs: allow format with user-specified uuid .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2926/ -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] fs: allow format with user-specified uuid
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3968 to look at the new patch set (#3). Change subject: fs: allow format with user-specified uuid .. fs: allow format with user-specified uuid The CLI tool is going to use this for the "handling permanent failure" workflow. Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c --- M src/kudu/fs/fs_manager-test.cc M src/kudu/fs/fs_manager.cc M src/kudu/fs/fs_manager.h M src/kudu/util/CMakeLists.txt A src/kudu/util/oid_generator-test.cc M src/kudu/util/oid_generator.cc M src/kudu/util/oid_generator.h 7 files changed, 136 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/68/3968/3 -- To view, visit http://gerrit.cloudera.org:8080/3968 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib4189d6150a263b7dde304dd19a449d3c0ab6c8c Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Start a background thread to run ResultTracker GC
Adar Dembo has posted comments on this change. Change subject: Start a background thread to run ResultTracker GC .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Another pass on 0.10.0 release notes
Adar Dembo has posted comments on this change. Change subject: Another pass on 0.10.0 release notes .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/3979/2/docs/release_notes.adoc File docs/release_notes.adoc: Line 141: // TODO(dan) can we link to the docs once they're committed? Looks like this is the only remaining TODO. -- To view, visit http://gerrit.cloudera.org:8080/3979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4414bdebb15d976c025dfce5a3f2bda5768bd5a9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-526: use on-disk cmeta when loading existing master state
Adar Dembo has submitted this change and it was merged. Change subject: KUDU-526: use on-disk cmeta when loading existing master state .. KUDU-526: use on-disk cmeta when loading existing master state The cmeta is populated when creating all new master state; we can trust it when that master is restarted. Note: this is a precondition for the migration from single-node to multi-node master deployments. Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70 Reviewed-on: http://gerrit.cloudera.org:8080/3786 Reviewed-by: Todd Lipcon Tested-by: Kudu Jenkins --- M src/kudu/integration-tests/master_failover-itest.cc M src/kudu/integration-tests/master_replication-itest.cc M src/kudu/master/sys_catalog.cc M src/kudu/master/sys_catalog.h 4 files changed, 94 insertions(+), 30 deletions(-) Approvals: Todd Lipcon: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3786 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5b4c6d8b6adf696973445a6f9d1314ba9de27e70 Gerrit-PatchSet: 8 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Dan Burkert has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 15: (8 comments) http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/integration-tests/exactly_once_writes-itest.cc File src/kudu/integration-tests/exactly_once_writes-itest.cc: PS15, Line 84: cncurrently concurrently + period PS15, Line 86: word missing PS15, Line 95: For Extra word? Something is off in this sentence. http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/exactly_once_rpc-test.cc File src/kudu/rpc/exactly_once_rpc-test.cc: Line 40: unique_ptr request_id(new RequestIdPB()); So I know this is a little late to the game, but RpcController holding it's request id by unique_ptr is pretty odd. I can't figure out if it's because it's a heavyweight object, or because it's optional. Looks to me like it should be ~40 bytes + whatever object overhead. http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/result_tracker.h File src/kudu/rpc/result_tracker.h: Line 225: // Runs garbage collection on the results this result tracker is caching. I would change this to 'Runs time-based garbage collection...' to distinguish it from the other GC type. PS15, Line 307: MustGcRecordFunc func Ideally this would be a template param instead of a std::function so that it can get inlined (I imagine this is perf. sensitive since it's done under a spinlock). http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/rpc_header.proto File src/kudu/rpc/rpc_header.proto: Line 126: required int64 first_incomplete_seq_no = 3; > RequestIDPB was only added during this release cycle, so no issue with olde All of these fields should be optional, we shouldn't be introducing any more required proto fields. http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/tablet/tablet_bootstrap.cc File src/kudu/tablet/tablet_bootstrap.cc: Line 1253: || state == ResultTracker::RpcState::COMPLETED || on previous lines -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] KUDU-456 Implement AUTO FLUSH BACKGROUND flush mode
Alexey Serbin has posted comments on this change. Change subject: KUDU-456 Implement AUTO_FLUSH_BACKGROUND flush mode .. Patch Set 6: (7 comments) Thank you for the review! I posted a new version (patchset 7) because the LINT build failed for patchset 6, so I wanted to fix that. http://gerrit.cloudera.org:8080/#/c/3952/6/src/kudu/client/client.cc File src/kudu/client/client.cc: Line 400: return shared_ptr(); > I've traced this through, and I can't find any place that this can actually OK, that sounds reasonable. Do I understand correctly that you suggest to remove this check at all? http://gerrit.cloudera.org:8080/#/c/3952/6/src/kudu/client/client.h File src/kudu/client/client.h: PS6, Line 1029: be > s/be/become Done http://gerrit.cloudera.org:8080/#/c/3952/6/src/kudu/client/session-internal.cc File src/kudu/client/session-internal.cc: Line 30: DEFINE_int32(client_buffer_bytes_limit, 7 * 1024 * 1024, > I think gflags don't apply to the client, so these will not really be confi Why not? If we exposed them in the documentation, a user could link a binary with the library having those flags exposed. But I agree: making those accessible via the client API would be easier to use, otherwise those are left hidden. However, I don't like idea of mutators for there -- I would better set them in the constructor and keep then unchanged. Probably, those might be properties of some sort of configuration object. How do you like the following: I'll mark those as hidden flags, which we will use only for testing purposes. Later on, we can expose controls for those up to the client API level. Does this make sense? Line 59: KuduSession::Data::BackgroundFlusher::BackgroundFlusher( > formatting (no wrap on initial argument, and 4 spaces for initializer list) OK, will fix. PS6, Line 91: waterline > We typically use 'watermark' instead of 'waterline' (at least in the Java i That's fun, because originally I had put 'watermark' and then figured that that was not the best term for that: https://en.wikipedia.org/wiki/Watermark_(disambiguation) OK, I'll replace it with watermark. Line 98: messenger->ScheduleOnReactor( > This looks like it's flushing the current buffer if it's over the waterline Yes, the Java implementation is different -- it does not have batchers and other stuff. Also, here the waterline is about amount of freshly added operations, not total size of the pending operations. http://gerrit.cloudera.org:8080/#/c/3952/6/src/kudu/util/monotime.h File src/kudu/util/monotime.h: Line 133: friend MonoTime operator -(const MonoTime& t, const MonoDelta& delta); > Maybe split the changes to MonoTime into its own review. I added them here because the tests I added rely on those. All right, having that as a separate change will be better from codereview's point. Will separate into a stand-alone change. -- To view, visit http://gerrit.cloudera.org:8080/3952 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34905c30b3aad96f53cf7a1822b1cde6d25f33a8 Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
[kudu-CR] Start a background thread to run ResultTracker GC
Kudu Jenkins has posted comments on this change. Change subject: Start a background thread to run ResultTracker GC .. Patch Set 4: Build Started http://104.196.14.100/job/kudu-gerrit/2924/ -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Kudu Jenkins has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 16: Build Started http://104.196.14.100/job/kudu-gerrit/2925/ -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: No
[kudu-CR] Start a background thread to run ResultTracker GC
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3961 to look at the new patch set (#4). Change subject: Start a background thread to run ResultTracker GC .. Start a background thread to run ResultTracker GC Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c --- M src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/service_if.cc M src/kudu/server/server_base.cc 5 files changed, 54 insertions(+), 25 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/3961/4 -- To view, visit http://gerrit.cloudera.org:8080/3961 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c Gerrit-PatchSet: 4 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Hello Alexey Serbin, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3628 to look at the new patch set (#16). Change subject: Add time/watermark based garbage collection to ResultTracker .. Add time/watermark based garbage collection to ResultTracker This adds time and watermark based garbage collection to the ResultTracker. Regarding time GC, there are two ttl's, a client ttl and a response ttl. After the response ttl has elapsed, we garbage collect responses but the ResultTracker remembers that it doesn't know them, so if the client retries a request older than that it gets a meaningful error back, stating that the request is stale. After the client ttl period without hearing back from a client, we GC the client state entirely, meaning all requests from that client will be treated as new. Regarding watermark GC the algorithm is simple, we trust the client to tell us what's its lowest incomplete sequence number and we GC everything below that. This adds a simple test that makes sure this basically works, and adds a multithreaded test that runs GC at the same time as writes. NOTE: this does not wire the time-based garbage collection process into the server itself -- it's currently only triggered by the included tests. Original patch by David. Some changes by Todd. Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 --- M src/kudu/integration-tests/exactly_once_writes-itest.cc M src/kudu/rpc/CMakeLists.txt A src/kudu/rpc/exactly_once_rpc-test.cc M src/kudu/rpc/result_tracker.cc M src/kudu/rpc/result_tracker.h M src/kudu/rpc/retriable_rpc.h D src/kudu/rpc/rpc-stress-test.cc M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/service_if.cc M src/kudu/tablet/tablet_bootstrap.cc M src/kudu/tablet/transactions/transaction_driver.cc 11 files changed, 784 insertions(+), 423 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/3628/16 -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 16 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/service_if.cc File src/kudu/rpc/service_if.cc: Line 104: RpcContext* ctx = new RpcContext(call, > ah, good catch... I wonder why leak sanitizer builds are not catching this. oh.. it's some slightly weird design going on here -- ResultTracker itself will have responded to the RPC in these cases, and thus deleted the context itself. I added a comment to that effect since it does _look_ like a leak. -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Add time/watermark based garbage collection to ResultTracker
Todd Lipcon has posted comments on this change. Change subject: Add time/watermark based garbage collection to ResultTracker .. Patch Set 15: (7 comments) http://gerrit.cloudera.org:8080/#/c/3628/15//COMMIT_MSG Commit Message: Line 9: This adds time and watermark based garbage collection to the ResultTracker. > Nit: the line is too long. Done Line 26: multithreaded threaded test that runs GC at the same time as writes. > Nit: an extra 'threaded' Done http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/exactly_once_rpc-test.cc File src/kudu/rpc/exactly_once_rpc-test.cc: Line 502: FLAGS_remember_responses_ttl_ms = 100; > Is it OK that the modified flags are not returned back to their default val we already wrap all of our tests in google::FlagSaver since the KuduTest base class has a FlagSaver member. I think some older tests that predate that addition have their own FlagSavers - could probably remove them now. Line 540: // This test creates a thread continuously makes requests to the server, some lasting longer > Nit: 'continuously makes' ---> 'continuously making' Done Line 548: FLAGS_remember_responses_ttl_ms = 10; > Ditto: consider adding google::FlagSaver to restore default values for the same http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/rpc_header.proto File src/kudu/rpc/rpc_header.proto: Line 126: required int64 first_incomplete_seq_no = 3; > Any concerns on backward compatibility here? If yes, does it make sense to RequestIDPB was only added during this release cycle, so no issue with older versions. The whole RequestIDPB itself is optional in the RPC header http://gerrit.cloudera.org:8080/#/c/3628/15/src/kudu/rpc/service_if.cc File src/kudu/rpc/service_if.cc: Line 104: RpcContext* ctx = new RpcContext(call, > Wouldn't it be a memory leak if the state was one of COMPLETED, IN_PROGRESS ah, good catch... I wonder why leak sanitizer builds are not catching this. Will investigate. -- To view, visit http://gerrit.cloudera.org:8080/3628 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c8e7b7191ca14842a31b64813ed498bdf626fa8 Gerrit-PatchSet: 15 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: David Ribeiro Alves Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: David Ribeiro Alves Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-HasComments: Yes
[kudu-CR] Improvements and corrections to Impala CREATE TABLE examples
Misty Stanley-Jones has submitted this change and it was merged. Change subject: Improvements and corrections to Impala CREATE TABLE examples .. Improvements and corrections to Impala CREATE TABLE examples Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c Reviewed-on: http://gerrit.cloudera.org:8080/3376 Reviewed-by: Dan Burkert Tested-by: Kudu Jenkins --- M docs/kudu_impala_integration.adoc 1 file changed, 36 insertions(+), 30 deletions(-) Approvals: Dan Burkert: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/3376 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I093972a7b806787a8c72634851796eebb5e1ae4c Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Misty Stanley-Jones Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR] KUDU-1517 Implement doc feedback from Sue M
Misty Stanley-Jones has posted comments on this change. Change subject: KUDU-1517 Implement doc feedback from Sue M .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3638/1/docs/index.adoc File docs/index.adoc: PS1, Line 85: > , (Oxford comma :-) Done http://gerrit.cloudera.org:8080/#/c/3638/1/docs/release_notes.adoc File docs/release_notes.adoc: Line 31: == Introduction > nit: I think this should be a 3rd-level heading Done -- To view, visit http://gerrit.cloudera.org:8080/3638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a7647b3e5d4d36e82e06ce02a45a8811e4efed3 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Misty Stanley-Jones Gerrit-Reviewer: Anonymous Coward #149 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: Yes
[kudu-CR] KUDU-1517 Implement doc feedback from Sue M
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/3638 to look at the new patch set (#3). Change subject: KUDU-1517 Implement doc feedback from Sue M .. KUDU-1517 Implement doc feedback from Sue M Change-Id: I8a7647b3e5d4d36e82e06ce02a45a8811e4efed3 --- M docs/index.adoc M docs/release_notes.adoc 2 files changed, 55 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/3638/3 -- To view, visit http://gerrit.cloudera.org:8080/3638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I8a7647b3e5d4d36e82e06ce02a45a8811e4efed3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Misty Stanley-Jones Gerrit-Reviewer: Anonymous Coward #149 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones
[kudu-CR] KUDU-1517 Implement doc feedback from Sue M
Kudu Jenkins has posted comments on this change. Change subject: KUDU-1517 Implement doc feedback from Sue M .. Patch Set 3: Build Started http://104.196.14.100/job/kudu-gerrit/2923/ -- To view, visit http://gerrit.cloudera.org:8080/3638 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8a7647b3e5d4d36e82e06ce02a45a8811e4efed3 Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Misty Stanley-Jones Gerrit-Reviewer: Anonymous Coward #149 Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Misty Stanley-Jones Gerrit-HasComments: No
