Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17326 )
Change subject: [tests] enable using txns in TestWorkload ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.h File src/kudu/integration-tests/test_workload.h: http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.h@85 PS1, Line 85: set_begin_txn() > nit: maybe, enforce this invariant with CHECK()? Another nit: it seems it' Updated the set_txn() calls to not take a bool. Also the invariants are checked at Setup(). http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.h@87 PS1, Line 87: int64_t txn_id > Since TestWorkload is most commonly used in more 'integrated' test scenario We're already using transaction tokens in that we deserialize tokens already. If you mean allow users to pass in a token, I don't want to put the burden on TestWorkload users to generate the token -- I'd rather make the interface simple, and use tokens under the hood, which we're doing already. http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.h@99 PS1, Line 99: // Commit the transaction that this workload is a part of upon calling : // StopAndJoin(). If set, either set_begin_txn() or set_txn_id() must be set : // as well. > nit: it would be nice to explicitly mention what happens if set_commit_txn( Done http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.h@102 PS1, Line 102: set_commit_txn > Does it make sense to add a method to roll back the transaction explicitly Done http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.cc File src/kudu/integration-tests/test_workload.cc: http://gerrit.cloudera.org:8080/#/c/17326/1/src/kudu/integration-tests/test_workload.cc@265 PS1, Line 265: // XXX(awong): this probably breaks. > Yup, it seems txn-enabled workload limits the available set of row test pat Removed the verification here for transaction workloads. -- To view, visit http://gerrit.cloudera.org:8080/17326 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia81daac8fcfd552603a0302c3d9aa411ea082ab1 Gerrit-Change-Number: 17326 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 29 Apr 2021 05:00:41 +0000 Gerrit-HasComments: Yes
