[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. Patch Set 4: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG@16 PS2, Line 16: Before this patch, CatalogManager returned Status::InvalidArgument() : for exact duplicates and otherwise overlapped ranges as well. > Right -- in case of exact range match the client will now get Status::Alrea Ok, sounds good to me as long as we document it in the release notes. -- 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: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 06 Oct 2020 18:17:37 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. [catalog_manager] Status::AlreadyPresent for range duplicates With this patch, CatalogManager discriminates between overlapped and exact duplicate key ranges when adding new partitions, returning Status::AlreadyPresent() for exact range duplicates and Status::InvalidArgument for otherwise overlapped ones. The number of hash buckets is not taken into account when searching/reporting about the exact duplicates in key range for partitions. Before this patch, CatalogManager returned Status::InvalidArgument() for exact duplicates and otherwise overlapped ranges as well. This patch also updates the relevant tests, so the new behavior has enough test coverage. A follow-up patch will use the newly introduced finer status reporting. To be more precise, TxnManager could send multiple requests to add the same new tablet for the transaction status table while processing concurrent BeginTransaction() requests. With that, it should be able to tell between an error when the corresponding tablet is already present and other types of errors. Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Reviewed-on: http://gerrit.cloudera.org:8080/16544 Tested-by: Kudu Jenkins Reviewed-by: Andrew Wong --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/catalog_manager.cc 4 files changed, 148 insertions(+), 102 deletions(-) Approvals: Kudu Jenkins: Verified Andrew Wong: Looks good to me, approved -- 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: merged Gerrit-Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Gerrit-Change-Number: 16544 Gerrit-PatchSet: 4 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. Patch Set 3: Thank you for the review! -- 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: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 06 Oct 2020 17:59:50 + Gerrit-HasComments: No
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
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 3: Code-Review+2 (3 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 > Right -- the idea is to make sure that's TxnManager who adds the partitions Ack 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) > See lines 2009 and 2041 of PS2 for the coverage of such cases. Ah, thanks for the pointer! 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: elements of 'existing_ta > Done Thanks! -- 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: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 06 Oct 2020 17:51:00 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16544 to look at the new patch set (#3). Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. [catalog_manager] Status::AlreadyPresent for range duplicates With this patch, CatalogManager discriminates between overlapped and exact duplicate key ranges when adding new partitions, returning Status::AlreadyPresent() for exact range duplicates and Status::InvalidArgument for otherwise overlapped ones. The number of hash buckets is not taken into account when searching/reporting about the exact duplicates in key range for partitions. Before this patch, CatalogManager returned Status::InvalidArgument() for exact duplicates and otherwise overlapped ranges as well. This patch also updates the relevant tests, so the new behavior has enough test coverage. A follow-up patch will use the newly introduced finer status reporting. To be more precise, TxnManager could send multiple requests to add the same new tablet for the transaction status table while processing concurrent BeginTransaction() requests. With that, it should be able to tell between an error when the corresponding tablet is already present and other types of errors. Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/catalog_manager.cc 4 files changed, 148 insertions(+), 102 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/16544/3 -- 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: newpatchset Gerrit-Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Gerrit-Change-Number: 16544 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG@16 PS2, Line 16: Before this patch, CatalogManager returned Status::InvalidArgument() : for exact duplicates and otherwise overlapped ranges as well. > Seems client will expect to see different error status being returned befor Right -- in case of exact range match the client will now get Status::AlreadyExists() instead of Status::InvaligArgument(). As I can see from the client.h, the exact meaning of the return codes hasn't been documented, but I guess it will be necessary to document this change in the release notes. I don't see this as a detrimental change -- instead, this patch improves the interface for this particular use case since Status::InvalidArgument() returned for other things like missing alter specification for table and column schema, invalid specification of key columns, etc. http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG@25 PS2, Line 25: concurrent BeginTransaction() requests. With that, it should be able > Sure, I guess I'm trying to piece together how this will be used by the Txn Right -- the idea is to make sure that's TxnManager who adds the partitions, and that will work even if TxnManagers request not equally-sized partitions, but just partitions of the same size to accommodate particular transaction identifier. That logic is already present in WIP patch: https://gerrit.cloudera.org/#/c/16527/1/src/kudu/master/txn_manager.cc@154 I revved that patch and added a test to calls TxnManager::BeginTransaction() concurrently, and found I need finer status reporting because Status::InvalidArgument() is too broad for this use case given the number of places in the AlterTable RPC's control flow which could result in Status::InvalidArgument(). I think TxnManager() and other use cases will benefit from this clear-cut semantics when concurrent actors request to create the same range partition. 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 ha See lines 2009 and 2041 of PS2 for the coverage of such cases. 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 specifi Yup. In addition, this change makes detecting the condition of already existing tablet with exact ranges much cleaner. 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 existin Done -- 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 Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Tue, 06 Oct 2020 04:11:52 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
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 Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Mon, 05 Oct 2020 23:29:49 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16544/2//COMMIT_MSG@16 PS2, Line 16: Before this patch, CatalogManager returned Status::InvalidArgument() : for exact duplicates and otherwise overlapped ranges as well. Seems client will expect to see different error status being returned before and after the change. Wondering do you foresee will this raise any compatibility concerns? -- 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 Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Mon, 05 Oct 2020 22:52:02 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16544 ) Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. Patch Set 2: (1 comment) 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 > Curious why we wouldn't have been able to just change all of these to Alrea At least from the point of pure semantics, returning AlreadyPresent for duplicate ranges is much cleaner, IMO. From that perspective, InvalidArgument is some sort of a broader status code, but this is not safe to assume otherwise. -- 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 Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Mon, 05 Oct 2020 22:29:40 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
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: Tagging Mahesh just for more exposure to more range partitioning code. -- 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 Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Comment-Date: Mon, 05 Oct 2020 22:07:10 + Gerrit-HasComments: No
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
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: (1 comment) 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 Curious why we wouldn't have been able to just change all of these to AlreadyPresent? I.e. why do we need to know about an exact match, vs knowing about range overlap in general? -- 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 Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Oct 2020 22:06:23 + Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16544 to look at the new patch set (#2). Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. [catalog_manager] Status::AlreadyPresent for range duplicates With this patch, CatalogManager discriminates between overlapped and exact duplicate key ranges when adding new partitions, returning Status::AlreadyPresent() for exact range duplicates and Status::InvalidArgument for otherwise overlapped ones. The number of hash buckets is not taken into account when searching/reporting about the exact duplicates in key range for partitions. Before this patch, CatalogManager returned Status::InvalidArgument() for exact duplicates and otherwise overlapped ranges as well. This patch also updates the relevant tests, so the new behavior has enough test coverage. A follow-up patch will use the newly introduced finer status reporting. To be more precise, TxnManager could send multiple requests to add the same new tablet for the transaction status table while processing concurrent BeginTransaction() requests. With that, it should be able to tell between an error when the corresponding tablet is already present and other types of errors. Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 --- M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/catalog_manager.cc 4 files changed, 137 insertions(+), 99 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/16544/2 -- 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: newpatchset Gerrit-Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Gerrit-Change-Number: 16544 Gerrit-PatchSet: 2 Gerrit-Owner: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [catalog manager] Status::AlreadyPresent for range duplicates
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16544 Change subject: [catalog_manager] Status::AlreadyPresent for range duplicates .. [catalog_manager] Status::AlreadyPresent for range duplicates With this patch, CatalogManager discriminates between overlapped and exact duplicate key ranges when adding new partitions, returning Status::AlreadyPresent() for exact range duplicates and Status::InvalidArgument for otherwise overlapped ones. The number of hash buckets is not taken into account when searching/reporting about the exact duplicates in key range for partitions. Before this patch, CatalogManager returned Status::InvalidArgument() for exact duplicates and otherwise overlapped ranges as well. This patch also updates the relevant tests, so the new behavior has enough test coverage. A follow-up patch will use the newly introduced finer status reporting. To be more precise, TxnManager could send multiple requests to add the same new tablet for the transaction status table while processing concurrent BeginTransaction() requests. With that, it should be able to tell between an error when the corresponding tablet is already present and other types of errors. Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 --- M src/kudu/integration-tests/alter_table-test.cc M src/kudu/integration-tests/txn_status_table-itest.cc M src/kudu/master/catalog_manager.cc 3 files changed, 128 insertions(+), 90 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/44/16544/1 -- 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: newchange Gerrit-Change-Id: I42c1b821f3bd6854c06682ae3c85a058665f1489 Gerrit-Change-Number: 16544 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
