[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16729 to look at the new patch set (#10). Change subject: KUDU-2612 keep-alive tracking for transactions .. KUDU-2612 keep-alive tracking for transactions This patch introduces the functionality of tracking the liveness of a distributed multi-row transaction into TxnStatusManager and provides corresponding proxy methods in TxnManager, so a Kudu client now can send keep-alive requests for a transaction (the implementation of the latter is planned in a follow-up patch). >From the TxnStatusManager, the newly introduced keep-alive RPC is represented as another type of CoordinateTransaction() request: CoordinatorOpPB::KEEP_TXN_ALIVE. New tests to cover the existing functionality are added as well. More end-to-end tests will be added by follow-up changelist once Kudu C++ client starts sending keepalive requests for started transactions. Also, all newly introduced tests in txn_status_manager-itest.cc are disabled because without [1] they are a bit flaky. I added TODO to re-enable those once [1] is committed. [1] https://gerrit.cloudera.org/#/c/16648/ Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/ts_tablet_manager-itest.cc A src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/txn_manager-test.cc M src/kudu/master/txn_manager.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_status_tablet.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 18 files changed, 1,032 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/16729/10 -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16729 )
Change subject: KUDU-2612 keep-alive tracking for transactions
..
Patch Set 9:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16729/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16729/9//COMMIT_MSG@9
PS9, Line 9: liviness
> nit: liveness
Done
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/integration-tests/txn_status_manager-itest.cc
File src/kudu/integration-tests/txn_status_manager-itest.cc:
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/integration-tests/txn_status_manager-itest.cc@373
PS9, Line 373: for (auto i = 0; i < 5; ++i) {
> We discussed this on Slack that there is flakiness here since new leaders d
Thank you for the reminder. I decided to comment this part out for a while,
adding a TODO to uncomment it after https://gerrit.cloudera.org/#/c/16648/
lands.
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@69
PS9, Line 69: the task is not run, meaning the TxnStatusManager doesn't "
: "automatically abort any stale/abandoned
transactions.
> This aspect of the flag isn't runtime is it? If not, we should clarify that
Yup, it seems that was change in the middle: there was no check to disable the
staleness detection task neither there was a logic to enable/disable those
check. I updated the code and this comment as needed.
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@401
PS9, Line 401: return Status::IllegalState(
> Actually I think this is correct as is, since this isn't a replicated call
Right: client doesn't use that logic of the retriable RPC from
coordinator_rpc.cc, it uses a different one from txn_manager_proxy_rpc.cc,
where IllegalState() is a non-retriable error.
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@462
PS9, Line 462: // Allow safety margin for receving and processing keep-alive
heartbeats
: // (half of the keepalive interval).
> nit: remove this now that it's no longer true?
Done
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@521
PS9, Line 521: Substitute(
> nit: remove Substitute?
Done
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@524
PS9, Line 524: return;
> Would it also make sense to check whether the replica is still running, and
Right -- I guess a deleted replica would issue those warnings. Added check
CheckRunning() verification step. It's a good idea to add the test scenario
you described because the transaction status tablet might be re-replicated, so
it's nice to have a coverage for the case when a txn status tablet replica is
suddenly not running when this code is executed. I think to add such a test
with in https://gerrit.cloudera.org/#/c/16779/ since it fits better to the test
harness which uses automatic txn keepalive from a client for a started
transaction.
http://gerrit.cloudera.org:8080/#/c/16729/5/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16729/5/src/kudu/tserver/ts_tablet_manager.cc@1372
PS5, Line 1372: true) {
: // Wait for a notification on shutdown or a timeout
expiration.
: if (shutdown_latch_.WaitFor(
:
MonoDelta::FromMilliseconds(FLAGS_txn_staleness_tracker_interval_ms))) {
: return;
: }
> I meant as:
I added there some logic, so while (true) looks like a more readable approach.
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/tserver/ts_tablet_manager.cc
File src/kudu/tserver/ts_tablet_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/tserver/ts_tablet_manager.cc@1375
PS9, Line 1375: FLAGS_txn_staleness_tracker_interval_ms
> Is this a tight loop if this is 0?
Good catch -- I guess the idea was not to schedule this task in such a case at
all, but then something has changes in the plans when
--txn_staleness_tracker_interval_ms became run-time flag.
--
To view, visit http://gerrit.cloudera.org:8080/16729
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175
Gerrit-Change-Number: 16729
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 10 Dec 2020 08:21:07 +
Gerrit-HasComments: Yes
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16853 Change subject: [thirpdarty] Improve LLVM build for macOS .. [thirpdarty] Improve LLVM build for macOS This patch adjusts the LLVM build definition to disable simulator platoforms on OSX that we do not support. This solves the issue that was worked around via 6597422 in a way that does not prevent building on aarch64 architectures. Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 --- M thirdparty/build-definitions.sh 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/16853/1 -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16852 Change subject: Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK .. Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK This is a follow on to 19c0f7b which introduced version checks to handle the introduction of preadv/pwritev in the OSX 11 SDK. Instead of using a compile time macro to disable using the preadv/pwritev simulations, this patch reverts to using them all the time and adjusts the naming of the method to ensure there are no symbol issues. This is important so that a binary compiled using the OSX 11 SDK can run on a machine running OSX < 11. This scenario will frequently occur on Xcode 12 given the OSX 11 SDK is installed by default and chosen by default by CMake even though the machine is running OSX 10. Change-Id: I90ba8f65b756c77c25dc776d0174b0d7b5678137 --- M src/kudu/util/env_posix.cc 1 file changed, 12 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/16852/1 -- To view, visit http://gerrit.cloudera.org:8080/16852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I90ba8f65b756c77c25dc776d0174b0d7b5678137 Gerrit-Change-Number: 16852 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] [build] Add the option to use the protoc from thirdparty in the java build
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16854 Change subject: [build] Add the option to use the protoc from thirdparty in the java build .. [build] Add the option to use the protoc from thirdparty in the java build This patch introduces the option to use the thirdparty protoc in the java build via `-PuseKuduProtoc`. This is useful if a specific OS or architecture is not published to Maven. Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f ex: `gradle assemble -PuseKuduProtoc` --- M java/gradle/dependencies.gradle M java/gradle/protobuf.gradle 2 files changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/54/16854/1 -- To view, visit http://gerrit.cloudera.org:8080/16854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f Gerrit-Change-Number: 16854 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16729 to look at the new patch set (#11). Change subject: KUDU-2612 keep-alive tracking for transactions .. KUDU-2612 keep-alive tracking for transactions This patch introduces the functionality of tracking the liveness of a distributed multi-row transaction into TxnStatusManager and provides corresponding proxy methods in TxnManager, so a Kudu client now can send keep-alive requests for a transaction (the implementation of the latter is planned in a follow-up patch). >From the TxnStatusManager, the newly introduced keep-alive RPC is represented as another type of CoordinateTransaction() request: CoordinatorOpPB::KEEP_TXN_ALIVE. New tests to cover the existing functionality are added as well. More end-to-end tests will be added by follow-up changelist once Kudu C++ client starts sending keepalive requests for started transactions. Also, all newly introduced tests in txn_status_manager-itest.cc are disabled because without [1] they are a bit flaky. I added TODO to re-enable those once [1] is committed. [1] https://gerrit.cloudera.org/#/c/16648/ Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/ts_tablet_manager-itest.cc A src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/txn_manager-test.cc M src/kudu/master/txn_manager.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_status_tablet.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 18 files changed, 1,034 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/16729/11 -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 11 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16853 ) Change subject: [thirpdarty] Improve LLVM build for macOS .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/16853/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16853/1//COMMIT_MSG@10 PS1, Line 10: platoforms typo http://gerrit.cloudera.org:8080/#/c/16853/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/16853/1/thirdparty/build-definitions.sh@a348 PS1, Line 348: I didn't understand removal of this line. http://gerrit.cloudera.org:8080/#/c/16853/1/thirdparty/build-definitions.sh@288 PS1, Line 288: buld typo -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:06:32 + Gerrit-HasComments: Yes
[kudu-CR] [build] Add the option to use the protoc from thirdparty in the java build
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16854 ) Change subject: [build] Add the option to use the protoc from thirdparty in the java build .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/16854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f Gerrit-Change-Number: 16854 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:08:47 + Gerrit-HasComments: No
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16853 ) Change subject: [thirpdarty] Improve LLVM build for macOS .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/16853/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/16853/1/thirdparty/build-definitions.sh@a348 PS1, Line 348: > I didn't understand removal of this line. It's not needed anymore. The previous issue we saw was a result of multiple aarch64 targets, but the above change to remove the various simulators is a permanent fix. We can now revert back to letting the architectures be dictated by the host. -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:11:14 + Gerrit-HasComments: Yes
[kudu-CR] Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16852 ) Change subject: Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/16852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90ba8f65b756c77c25dc776d0174b0d7b5678137 Gerrit-Change-Number: 16852 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:15:52 + Gerrit-HasComments: No
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/16853 ) Change subject: [thirpdarty] Improve LLVM build for macOS .. Patch Set 1: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:52:32 + Gerrit-HasComments: No
[kudu-CR] [build] Add the option to use the protoc from thirdparty in the java build
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/16854 ) Change subject: [build] Add the option to use the protoc from thirdparty in the java build .. Patch Set 1: What's an example of a specific OS or arch missing from Maven that applies to Kudu? -- To view, visit http://gerrit.cloudera.org:8080/16854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f Gerrit-Change-Number: 16854 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 18:53:56 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/16729 ) Change subject: KUDU-2612 keep-alive tracking for transactions .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 11 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16729 ) Change subject: KUDU-2612 keep-alive tracking for transactions .. Patch Set 11: Verified+1 Unrelated test failures in: * BloomFilterPredicateTest.TestKuduBloomFilterPredicateBenchmark * TableLocationsCacheMultiMasterTest.ResetCache -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 11 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 10 Dec 2020 19:09:59 + Gerrit-HasComments: No
[kudu-CR] [build] Add the option to use the protoc from thirdparty in the java build
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16854 ) Change subject: [build] Add the option to use the protoc from thirdparty in the java build .. Patch Set 1: osx arm64 for the time being -- To view, visit http://gerrit.cloudera.org:8080/16854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f Gerrit-Change-Number: 16854 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 20:24:52 + Gerrit-HasComments: No
[kudu-CR] [build] Add the option to use the protoc from thirdparty in the java build
Greg Solovyev has posted comments on this change. ( http://gerrit.cloudera.org:8080/16854 ) Change subject: [build] Add the option to use the protoc from thirdparty in the java build .. Patch Set 1: Code-Review+1 > osx arm64 for the time being Thanks. -- To view, visit http://gerrit.cloudera.org:8080/16854 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I64bdf94c222c971b4900213d58f25c50c860ec9f Gerrit-Change-Number: 16854 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 20:33:36 + Gerrit-HasComments: No
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Hello Kudu Jenkins, Andrew Wong, Greg Solovyev, Bankim Bhavsar, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16853 to look at the new patch set (#2). Change subject: [thirpdarty] Improve LLVM build for macOS .. [thirpdarty] Improve LLVM build for macOS This patch adjusts the LLVM build definition to disable simulator platforms on OSX that we do not support. This solves the issue that was worked around via 6597422 in a way that does not prevent building on aarch64 architectures. Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 --- M thirdparty/build-definitions.sh 1 file changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/53/16853/2 -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/16779/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16779/3//COMMIT_MSG@14 PS3, Line 14: goes : out of scope. In contrast, if the transaction handle is created by : KuduTransaction::Deserialize(), the keepalive messages are or aren't > Hmm, yeah, I think that should be sufficient. I also added a small blurb about the into the in-line doc for KuduTransactionSerializer. http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/client.h@444 PS3, Line 444: /// : /// An > nit: A Done http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc File src/kudu/client/transaction-internal.cc: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc@357 PS3, Line 357: DCHECK(status.IsAborted()); > nit: could you add a comment adding some insight into where these errors ma Done http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc@387 PS3, Line 387: req > This gave me pause because it's scoped to be destructed at the end of the t Right: the information from 'req' is used only in SendRpc, and once SendRpc() returns, it's no longer needed. From the other side, this look strange, indeed. I guess the better signature should just use move semantics for this parameter. I'm planning to post a follow-up patch to update this. -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 10 Dec 2020 23:44:34 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Hello Kudu Jenkins, Zoltan Borok-Nagy, Andrew Wong, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16779 to look at the new patch set (#4). Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. KUDU-2612 keep-alive txn heartbeating for C++ client This patch adds keep-alive txn heartbeating into the Kudu C++ client. The txn keepalive heartbeating is performed automatically by the client, and no API is exposed to send keep-alive messages for a transaction. The txn keepalive heartbeating continues until the original transaction handle (i.e. the handle created by KuduClient::NewTransaction()) goes out of scope. In contrast, if the transaction handle is created by KuduTransaction::Deserialize(), the keepalive messages are or aren't sent depending on the KuduTransactionSerializer::enable_keepalive() setting when serializing the source handle. By default, keepalive messages are not sent for deserialized transaction handles. This is because the most common use case for multiple actors working in the context of the same transaction is supposed to be of the "star topology", when a transaction is started and committed by a top-level application who shares the context of the transaction with other applications which only submit their data, but don't manage the lifecycle of the transaction. This patch also contains a couple of test scenarios to cover the newly introduced functionality. Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h M src/kudu/client/txn_manager_proxy_rpc.cc M src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/transactions/transactions.proto 8 files changed, 495 insertions(+), 63 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16779/4 -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Hello Kudu Jenkins, Zoltan Borok-Nagy, Andrew Wong, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16779 to look at the new patch set (#5). Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. KUDU-2612 keep-alive txn heartbeating for C++ client This patch adds keep-alive txn heartbeating into the Kudu C++ client. The txn keepalive heartbeating is performed automatically by the client, and no API is exposed to send keep-alive messages for a transaction. The txn keepalive heartbeating continues until the original transaction handle (i.e. the handle created by KuduClient::NewTransaction()) goes out of scope. In contrast, if the transaction handle is created by KuduTransaction::Deserialize(), the keepalive messages are or aren't sent depending on the KuduTransactionSerializer::enable_keepalive() setting when serializing the source handle. By default, keepalive messages are not sent for deserialized transaction handles. This is because the most common use case for multiple actors working in the context of the same transaction is supposed to be of the "star topology", when a transaction is started and committed by a top-level application who shares the context of the transaction with other applications which only submit their data, but don't manage the lifecycle of the transaction. This patch also contains a couple of test scenarios to cover the newly introduced functionality. Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h M src/kudu/client/txn_manager_proxy_rpc.cc M src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/transactions/transactions.proto 8 files changed, 495 insertions(+), 63 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16779/5 -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 5 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[kudu-CR] [thirpdarty] Improve LLVM build for macOS
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16853 ) Change subject: [thirpdarty] Improve LLVM build for macOS .. Patch Set 2: Code-Review+2 Thank you for fixing this! I haven't tried to build on macOS HighSierra (10.13), but I guess should works if it works on newer OS versions. -- To view, visit http://gerrit.cloudera.org:8080/16853 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I45c60cd2da120906c436b5592ad6b9e6a89dde66 Gerrit-Change-Number: 16853 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Greg Solovyev Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 23:52:09 + Gerrit-HasComments: No
[kudu-CR] Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16852 ) Change subject: Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK .. Patch Set 1: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/16852/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16852/1//COMMIT_MSG@9 PS1, Line 9: 19c0f7b I'm not sure this is the correct git hash for the actual changelist: https://github.com/apache/kudu/commit/19c0f7b19b623bc91ebcd7b6451300dc45364b0b ? -- To view, visit http://gerrit.cloudera.org:8080/16852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90ba8f65b756c77c25dc776d0174b0d7b5678137 Gerrit-Change-Number: 16852 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 10 Dec 2020 23:57:21 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Hello Kudu Jenkins, Zoltan Borok-Nagy, Andrew Wong, Hao Hao, Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16779 to look at the new patch set (#6). Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. KUDU-2612 keep-alive txn heartbeating for C++ client This patch adds keep-alive txn heartbeating into the Kudu C++ client. The txn keepalive heartbeating is performed automatically by the client, and no API is exposed to send keep-alive messages for a transaction. The txn keepalive heartbeating continues until the original transaction handle (i.e. the handle created by KuduClient::NewTransaction()) goes out of scope. In contrast, if the transaction handle is created by KuduTransaction::Deserialize(), the keepalive messages are or aren't sent depending on the KuduTransactionSerializer::enable_keepalive() setting when serializing the source handle. By default, keepalive messages are not sent for deserialized transaction handles. This is because the most common use case for multiple actors working in the context of the same transaction is supposed to be of the "star topology", when a transaction is started and committed by a top-level application who shares the context of the transaction with other applications which only submit their data, but don't manage the lifecycle of the transaction. This patch also contains a couple of test scenarios to cover the newly introduced functionality. Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 --- M src/kudu/client/client-test.cc M src/kudu/client/client.cc M src/kudu/client/client.h M src/kudu/client/transaction-internal.cc M src/kudu/client/transaction-internal.h M src/kudu/client/txn_manager_proxy_rpc.cc M src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/transactions/transactions.proto 8 files changed, 495 insertions(+), 63 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/16779/6 -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/16729 )
Change subject: KUDU-2612 keep-alive tracking for transactions
..
Patch Set 11: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16729/11/src/kudu/transactions/txn_status_manager-test.cc
File src/kudu/transactions/txn_status_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16729/11/src/kudu/transactions/txn_status_manager-test.cc@109
PS11, Line 109: void CheckForTxnStatus(
: int64_t txn_id, const string& txn_owner, TxnStatePB
expected_txn_state) {
: TxnStatusEntryPB status;
: TabletServerErrorPB ts_error;
: ASSERT_OK(txn_manager_->GetTransactionStatus(
: txn_id, txn_owner, &status, &ts_error));
: ASSERT_TRUE(status.has_state());
: ASSERT_EQ(expected_txn_state, status.state());
: }
nit: not used (anymore?)?
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc
File src/kudu/transactions/txn_status_manager.cc:
http://gerrit.cloudera.org:8080/#/c/16729/9/src/kudu/transactions/txn_status_manager.cc@524
PS9, Line 524: "last
> Right -- I guess a deleted replica would issue those warnings. Added check
SGTM
--
To view, visit http://gerrit.cloudera.org:8080/16729
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175
Gerrit-Change-Number: 16729
Gerrit-PatchSet: 11
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 11 Dec 2020 02:01:07 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Patch Set 6: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc File src/kudu/client/transaction-internal.cc: http://gerrit.cloudera.org:8080/#/c/16779/3/src/kudu/client/transaction-internal.cc@357 PS3, Line 357: // de-scheduled from the qu > Done Thanks! -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 11 Dec 2020 02:10:08 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Hello Tidy Bot, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16729 to look at the new patch set (#12). Change subject: KUDU-2612 keep-alive tracking for transactions .. KUDU-2612 keep-alive tracking for transactions This patch introduces the functionality of tracking the liveness of a distributed multi-row transaction into TxnStatusManager and provides corresponding proxy methods in TxnManager, so a Kudu client now can send keep-alive requests for a transaction (the implementation of the latter is planned in a follow-up patch). From the TxnStatusManager, the newly introduced keep-alive RPC is represented as another type of CoordinateTransaction() request: CoordinatorOpPB::KEEP_TXN_ALIVE. New tests to cover the existing functionality are added as well. More end-to-end tests will be added by follow-up changelist once Kudu C++ client starts sending keepalive requests for started transactions. Also, all newly introduced tests in txn_status_manager-itest.cc are disabled because without [1] they are a bit flaky. I added TODO to re-enable those once [1] is committed. [1] https://gerrit.cloudera.org/#/c/16648/ Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/ts_tablet_manager-itest.cc A src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/txn_manager-test.cc M src/kudu/master/txn_manager.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_status_tablet.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 18 files changed, 1,024 insertions(+), 59 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/16729/12 -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 12 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16729 )
Change subject: KUDU-2612 keep-alive tracking for transactions
..
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16729/11/src/kudu/transactions/txn_status_manager-test.cc
File src/kudu/transactions/txn_status_manager-test.cc:
http://gerrit.cloudera.org:8080/#/c/16729/11/src/kudu/transactions/txn_status_manager-test.cc@109
PS11, Line 109: void CheckForTxnStatus(
: int64_t txn_id, const string& txn_owner, TxnStatePB
expected_txn_state) {
: TxnStatusEntryPB status;
: TabletServerErrorPB ts_error;
: ASSERT_OK(txn_manager_->GetTransactionStatus(
: txn_id, txn_owner, &status, &ts_error));
: ASSERT_TRUE(status.has_state());
: ASSERT_EQ(expected_txn_state, status.state());
: }
> nit: not used (anymore?)?
Good catch -- indeed, this is no longer needed.
--
To view, visit http://gerrit.cloudera.org:8080/16729
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175
Gerrit-Change-Number: 16729
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 11 Dec 2020 03:10:58 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Removed reviewer Kudu Jenkins with the following votes: * Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Patch Set 6: Verified+1 unrelated test failure in ValidityIntervals/AuthTokenExpireDuringWorkloadITest.InvalidTokenDuringMixedWorkload/2 -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 11 Dec 2020 03:11:39 + Gerrit-HasComments: No
[kudu-CR] [server] add 'uptime' metric for server
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16858 Change subject: [server] add 'uptime' metric for server .. [server] add 'uptime' metric for server This patch adds a new 'uptime' metric for a Kudu server. The metric's value is reported as the duration of the time interval passed from the start of the server. The interval duration is reported in microseconds as many other time-related metrics. Initially I thought to reuse already existing field that stores server's start time as wall clock time in seconds, but such approach would not produce a reliable result given that the wall clock time can be set any time by a super user, hence a wall-clock-based delta could not reliably provide the actual uptime of a server. The motivation for this patch is the realization that sometimes it's not easy to interpret metric values reported for a server without knowing its uptime. I didn't add any tests for the newly introduced metric, but I manually verified that both kudu-master and kudu-tserver processes report uptime among their metrics at the "/metrics" HTTP endpoint. Change-Id: Idb362a71c625c05a2185b95ec24d3f371dcb4155 --- M src/kudu/integration-tests/registration-test.cc M src/kudu/master/master.cc M src/kudu/server/server_base.cc M src/kudu/server/server_base.h M src/kudu/tserver/heartbeater.cc 5 files changed, 31 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/16858/1 -- To view, visit http://gerrit.cloudera.org:8080/16858 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Idb362a71c625c05a2185b95ec24d3f371dcb4155 Gerrit-Change-Number: 16858 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[kudu-CR] Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16852 ) Change subject: Improve preadv/pwritev with Xcode 12 and the OSX 11 SDK .. Patch Set 1: Code-Review+1 (1 comment) I verified the broken tests are fixed with this patch with osx 10.5.7 and Xcode12. http://gerrit.cloudera.org:8080/#/c/16852/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16852/1//COMMIT_MSG@13 PS1, Line 13: using nit: use -- To view, visit http://gerrit.cloudera.org:8080/16852 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I90ba8f65b756c77c25dc776d0174b0d7b5678137 Gerrit-Change-Number: 16852 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Bankim Bhavsar Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 11 Dec 2020 04:13:02 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 keep-alive txn heartbeating for C++ client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16779 ) Change subject: KUDU-2612 keep-alive txn heartbeating for C++ client .. Patch Set 6: -Verified > unrelated test failure in > ValidityIntervals/AuthTokenExpireDuringWorkloadITest.InvalidTokenDuringMixedWorkload/2 There is on issue to fix: a warning is reported by TSAN: it's related to the client's lifecycle and handling keep-alive callback. I'll fix it. -- To view, visit http://gerrit.cloudera.org:8080/16779 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0283d8e16908f641388f7a30b513a672df27a186 Gerrit-Change-Number: 16779 Gerrit-PatchSet: 6 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Fri, 11 Dec 2020 04:14:54 + Gerrit-HasComments: No
[kudu-CR] [master] Range specific hashing at table creation time.
Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16859 Change subject: [master] Range specific hashing at table creation time. .. [master] Range specific hashing at table creation time. This patch updates CreateTableRequestPB to allow different hash schemas to be defined per range at table creation time. This new field is appropriately decoded in catalog_manager.cc. The changes to kudu/common include some refactoring and putting functions back into an anonymous namespace. Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d --- M src/kudu/common/partition.cc M src/kudu/common/partition.h M src/kudu/integration-tests/table_locations-itest.cc M src/kudu/master/catalog_manager.cc M src/kudu/master/master-test.cc M src/kudu/master/master.proto 6 files changed, 205 insertions(+), 48 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/16859/1 -- To view, visit http://gerrit.cloudera.org:8080/16859 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8f0dcbc3324f8f2d6e99b4d169fdf5c7f7dff95d Gerrit-Change-Number: 16859 Gerrit-PatchSet: 1 Gerrit-Owner: Mahesh Reddy
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16729 ) Change subject: KUDU-2612 keep-alive tracking for transactions .. Patch Set 12: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 12 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 11 Dec 2020 05:13:52 + Gerrit-HasComments: No
[kudu-CR] KUDU-2612 keep-alive tracking for transactions
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16729 ) Change subject: KUDU-2612 keep-alive tracking for transactions .. KUDU-2612 keep-alive tracking for transactions This patch introduces the functionality of tracking the liveness of a distributed multi-row transaction into TxnStatusManager and provides corresponding proxy methods in TxnManager, so a Kudu client now can send keep-alive requests for a transaction (the implementation of the latter is planned in a follow-up patch). >From the TxnStatusManager, the newly introduced keep-alive RPC is represented as another type of CoordinateTransaction() request: CoordinatorOpPB::KEEP_TXN_ALIVE. New tests to cover the existing functionality are added as well. More end-to-end tests will be added by follow-up changelist once Kudu C++ client starts sending keepalive requests for started transactions. Also, all newly introduced tests in txn_status_manager-itest.cc are disabled because without [1] they are a bit flaky. I added TODO to re-enable those once [1] is committed. [1] https://gerrit.cloudera.org/#/c/16648/ Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Reviewed-on: http://gerrit.cloudera.org:8080/16729 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M src/kudu/integration-tests/CMakeLists.txt M src/kudu/integration-tests/ts_tablet_manager-itest.cc A src/kudu/integration-tests/txn_status_manager-itest.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/txn_manager-test.cc M src/kudu/master/txn_manager.cc M src/kudu/tablet/txn_coordinator.h M src/kudu/transactions/txn_status_entry.cc M src/kudu/transactions/txn_status_entry.h M src/kudu/transactions/txn_status_manager-test.cc M src/kudu/transactions/txn_status_manager.cc M src/kudu/transactions/txn_status_manager.h M src/kudu/transactions/txn_status_tablet.h M src/kudu/transactions/txn_system_client.cc M src/kudu/transactions/txn_system_client.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/ts_tablet_manager.cc M src/kudu/tserver/ts_tablet_manager.h 18 files changed, 1,024 insertions(+), 59 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/16729 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iae926e02fa7ca597b63ccea90124964c3b6a1175 Gerrit-Change-Number: 16729 Gerrit-PatchSet: 13 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] tablet: allow interleaving of row liveness between compaction input rows
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16752 to look at the new patch set (#3). Change subject: tablet: allow interleaving of row liveness between compaction input rows .. tablet: allow interleaving of row liveness between compaction input rows It was previously true that when merging multiple rowsets, a row's liveness always moved to the same or newer rowsets. I.e., if a row was deleted and inserted again, the new insert would either land in the same rowset from which it was deleted (e.g. if deleting from the MRS and inserting back into the same MRS), or a rowset whose rows are newer than the rowset being deleted from (e.g. if deleting from a DRS and inserting into the MRS). This invariant was upheld by various checks in compaction code. The invariant is no longer true when considering transactional inserts. Take the following example: - ts=10: insert 'a' to the tablet's MRS - ts=11: delete 'a' from the tablet's MRS - begin txn - insert 'a' to a transactional MRS - ts=12: commit the transactional MRS - ts=13: delete 'a' from the transactional MRS - ts=14: insert 'a' to the tablet's MRS In considering the row history for 'a' in the context of defining the row's history, we're left with the following row histories in each rowset, which serve as an input to our history-merging code: tablet MRS: UNDO(del@10) <- 'a' -> REDO(del@11) -> REDO(reins@14) txn MRS:UNDO(del@12) <- 'a' -> REDO(del@13) Previously, our invariant meant that one of these rows entire history (both UNDOs and REDOs) was entirely ahead of the other. This meant that merging was a matter of determining which input is older (which must be a deleted "ghost" row, since there can only be a single live row at a time), converting the older row and its history into UNDOs, and merging that UNDO history with the newer row's UNDOs. Given the above sequence, defining an older row isn't as straightforward, given the overlapping of the histories. This led to broken invariants in the form of CHECK failures or incorrect results. However, a notable obvservation is that there _is_ a discernable history that does abide by our previously held invariant, if we transfer the newer REDOs from the tablet's MRS input to the txn's MRS input: UNDO(del@10) <- 'a' -> REDO(del@11) UNDO(del@12) <- 'a' -> REDO(del@13) -> REDO(reins@14) This transferral is safe because we still expect that only a single instance of a row can be "live" at a time. I.e., if there is such overlap, it is caused by the deletion of a row from the older rowset, and in transferring the history, there is no risk of ending up with two histories for the same row that both end with a live row. This patch implements this transferral of history onto the newer input, and adds some fuzz test cases that helped snuff out the aforementioned CHECK failures and incorrect results. It also enables transactional ops in fuzz-itest, which helped find this issue. Without this patch, fuzz-itest with transactional support fails 2/10 times in DEBUG mode. With it, it has passed 1000/1000 times. Change-Id: I042a7d70d32edf9d2a3a077790821893f162880a --- M src/kudu/integration-tests/fuzz-itest.cc M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/mutation.h 5 files changed, 491 insertions(+), 70 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/16752/3 -- To view, visit http://gerrit.cloudera.org:8080/16752 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I042a7d70d32edf9d2a3a077790821893f162880a Gerrit-Change-Number: 16752 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: fuzz transactional inserts
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16699 to look at the new patch set (#5). Change subject: KUDU-2612: fuzz transactional inserts .. KUDU-2612: fuzz transactional inserts This patch updates fuzz-itest to include transactionality to the test. Since transactionality keeps ops hidden until commit time, this meant adding some maps to keep track of new expected and pending state while ops were uncommitted. I also included a couple of test cases that were found to cause issues that were addressed in previous patches. For now, I've commented out the inclusion of transactional ops because I've found them to be flaky on account of a potential debug crash when merging transactional MRSs. This will be addressed in a follow-up commit, but I'd like to merge this first, as this test has been useful in testing the follow-up change. When fixed, however, the patches together passed fuzz-itest 1000/1000 times with slow tests enabled and disabled. Change-Id: I719d42327ab18fda874332c9d6e1ae34aca8e846 --- M src/kudu/client/client.h M src/kudu/integration-tests/fuzz-itest.cc 2 files changed, 569 insertions(+), 118 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/99/16699/5 -- To view, visit http://gerrit.cloudera.org:8080/16699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I719d42327ab18fda874332c9d6e1ae34aca8e846 Gerrit-Change-Number: 16699 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-2612: fuzz transactional inserts
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16699 ) Change subject: KUDU-2612: fuzz transactional inserts .. Patch Set 5: (10 comments) http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc File src/kudu/integration-tests/fuzz-itest.cc: http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@169 PS4, Line 169: "TEST_ABORT_TXN", > nit: Maybe `kNoVal` since not all ops use this value as a txn id. Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@277 PS4, Line 277: TEST_RESTART_TS, > Any reason not to test transaction related ops here? Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@787 PS4, Line 787: case TEST_INSERT_IGNORE: > More documentation. Awesome! Yea, jumping into this test without much context is pretty intimidating. Hope it helps! http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@949 PS4, Line 949: } > Dos this completely skip UPDATE_IGNORE ops for missing rows? Good catch! Totally borked the UPDATE_IGNORE path and some of the checking for setting data_in_dms. http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@978 PS4, Line 978: // The row is being operated on by an in-flight op, but the pending > Dos this completely skip DELETE_IGNORE ops for missing rows? Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@992 PS4, Line 992: } > Nit: Add a comment explaining this continue like the others. Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@1075 PS4, Line 1075: // this -- they only set 'data_in_mrs' once committed. > Nit: Add a comment explaining this continue like the others. Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@1134 PS4, Line 1134: } > warning: method 'ValidateFuzzCase' can be made static [readability-convert- Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@1154 PS4, Line 1154: ops->emplace_back(TEST_FLUSH_OPS, txn_id); > Mind adding a TODO for the future about handling UPSERT pending_rows_per_tx Done http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@1177 PS4, Line 1177: auto pending_existence = EraseKeyReturnValuePtr(&pending_existence_per_txn, txn_id); > Would it be worth also storing the OP type in the pending_rows_per_txn too Done -- To view, visit http://gerrit.cloudera.org:8080/16699 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I719d42327ab18fda874332c9d6e1ae34aca8e846 Gerrit-Change-Number: 16699 Gerrit-PatchSet: 5 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 11 Dec 2020 07:15:29 + Gerrit-HasComments: Yes
[kudu-CR] tablet: allow interleaving of row liveness between compaction input rows
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16752 ) Change subject: tablet: allow interleaving of row liveness between compaction input rows .. Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc File src/kudu/tablet/compaction.cc: http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@318 PS2, Line 318: // : // UNDO(del@5) <- BASE(a) -> REDO(del@10) -> REDO(reins@25) // main MR > Wondering what will happen if Addressed in the other comment. http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@390 PS2, Line 390: const Mutation* newer_row_last = left_newer ? left_last: right_last; > Just for my own understanding, wondering when this happen if all the ops ar They would all be assigned different timestamps, and we'd take the branch at L363. http://gerrit.cloudera.org:8080/#/c/16752/2/src/kudu/tablet/compaction.cc@453 PS2, Line 453: CopyMutations(Mutat > It seems for all REDOs in the 'older' that are greater than the REDO head o We discussed this on Slack, that I don't think it's a valid sequence in terms of how tablets generate mutations. For this to happen, the txn MRS would have to become "active" again at ts=40 and accept a write, but the transaction was committed already at ts=15 and thus stopped inserting new inserts. The delete at ts=20 was part of a different op, but landed in the transaction MRS because mutations attach themselves to where the latest live row is. I've added a comment explaining. Let me know if I can clarify more. -- To view, visit http://gerrit.cloudera.org:8080/16752 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I042a7d70d32edf9d2a3a077790821893f162880a Gerrit-Change-Number: 16752 Gerrit-PatchSet: 3 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Fri, 11 Dec 2020 07:15:36 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612: fuzz transactional inserts
Alexey Serbin has posted comments on this change. (
http://gerrit.cloudera.org:8080/16699 )
Change subject: KUDU-2612: fuzz transactional inserts
..
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc@441
PS3, Line 441: ey;
> I just found it easier to read since I'm better versed at math operations t
Makes sense :)
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc@786
PS3, Line 786:
> Done
Thank you for adding the explanation!
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc@868
PS3, Line 868:
> It is acceptable, e.g. if we have chosen to insert not as a part of a trans
Indeed -- I realized that after reading further, but it seems I forgot to
discard that comment. Thank you for the confirmation!
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc@1142
PS3, Line 1142: LLTHROUGH_INTENDE
> Done
Not sure I see that CHECK() was added?
http://gerrit.cloudera.org:8080/#/c/16699/3/src/kudu/integration-tests/fuzz-itest.cc@1147
PS3, Line 1147: exists[test_op.val] = true;
> Does it make sense to verify that the key was present in the container?
Not sure whether you missed this one or this doesn't make much sense?
http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc
File src/kudu/integration-tests/fuzz-itest.cc:
http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@277
PS4, Line 277: const vector kPkOnlyOps {TEST_INSERT_PK_ONLY,
> Any reason not to test transaction related ops here?
+1
http://gerrit.cloudera.org:8080/#/c/16699/4/src/kudu/integration-tests/fuzz-itest.cc@1734
PS4, Line 1734: {TEST_BEGIN_TXN, 0},
: {TEST_INSERT_IGNORE, 1, 0},
: {TEST_FLUSH_OPS, 0},
: {TEST_COMMIT_TXN, 0},
:
: {TEST_INSERT_PK_ONLY, 0, -1},
: {TEST_INSERT_IGNORE_PK_ONLY, 0, -1},
: {TEST_DELETE, 0},
: {TEST_FLUSH_OPS, -1},
Does it make sense to add TEST_UPDATE and MINOR_COMPACT_DELTAS into the
sequence, where UPDATE is on the affected row after committing the transaction?
Or that deserves its own fuzz sequence?
--
To view, visit http://gerrit.cloudera.org:8080/16699
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I719d42327ab18fda874332c9d6e1ae34aca8e846
Gerrit-Change-Number: 16699
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 11 Dec 2020 07:28:46 +
Gerrit-HasComments: Yes
