Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 )
Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG@25 PS2, Line 25: concurrent BeginTransaction() requests. With that, it should be able > At least from the point of pure semantics, returning AlreadyPresent for dup Sure, I guess I'm trying to piece together how this will be used by the TxnManager. I'm assuming something like: if we make a request and get back AlreadyPresent, a previous request (from this TxnManager or another one) has succeeded and we can proceed with the next txn ID. We never expect to get back InvalidArgument because we will assume that all TxnManagers will request equally-sized range blocks. Is that all correct? http://gerrit.cloudera.org:8080/#/c/16544/2/src/kudu/integration-tests/alter_table-test.cc File src/kudu/integration-tests/alter_table-test.cc: http://gerrit.cloudera.org:8080/#/c/16544/2/src/kudu/integration-tests/alter_table-test.cc@1420 PS2, Line 1420: // ADD [0, 100) <- already present (duplicate) What about the case where the upper or lower bounds don't exist (i.e. we have unbounded partitions)? http://gerrit.cloudera.org:8080/#/c/16544/2/src/kudu/integration-tests/alter_table-test.cc@1428 PS2, Line 1428: IsAlreadyPresent Looking around the client docs a bit, I don't think we document the specific error code, and so I suppose it's OK to change this error code. http://gerrit.cloudera.org:8080/#/c/16544/2/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/16544/2/src/kudu/master/catalog_manager.cc@2467 PS2, Line 2467: tablet directly *after* nit: could you clarify this comment to indicate which bounds of the existing_tablets its comparing against? E.g. is this comparing against the lower bounds of existing tablets? Also what does it mean for iter to equal existing_tablets.begin()? -- To view, visit http://gerrit.cloudera.org:8080/16544 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Gerrit-Change-Number: 16544 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Mon, 05 Oct 2020 23:29:49 +0000 Gerrit-HasComments: Yes
