Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/20270 )
Change subject: KUDU-3461 [client] Avoid impala crash by returning error if invalid tablet id found ...................................................................... Patch Set 12: (5 comments) http://gerrit.cloudera.org:8080/#/c/20270/10/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/20270/10/src/kudu/client/meta_cache.cc@1291 PS10, Line 1291: : Status MetaCache::FastLookupTabletByKey(const KuduTable* table, : PartitionKey partition_key, : MetaCache::LookupType lookup_type, : scoped_refptr<RemoteTablet>* remote_tablet) { : > All right: as I understand, the MetaCache isn't aware of the context of tab Tablet invalidation happens as part of lookup via slowpath when client asks server to lookup tablet as it fails to find its corresponding entry in metacache. RPC response receives RESOURCE_NOT_FOUND error to which RPC callback action is to mark the tablet (old entry sent by batcher) as stale. http://gerrit.cloudera.org:8080/#/c/20270/12/src/kudu/client/meta_cache.cc File src/kudu/client/meta_cache.cc: http://gerrit.cloudera.org:8080/#/c/20270/12/src/kudu/client/meta_cache.cc@542 PS12, Line 542: local > nit: why 'local'? Is there a 'remote' metacache as well? Removed 'local' to avoid confusion. There is just one metacache on client side. http://gerrit.cloudera.org:8080/#/c/20270/12/src/kudu/client/meta_cache.cc@542 PS12, Line 542: tablet > nit: the tablet ? Done. http://gerrit.cloudera.org:8080/#/c/20270/12/src/kudu/client/meta_cache.cc@552 PS12, Line 552: tablet_id().empty() > I'm curious: is it even possible to have tablet_id empty? If yes, what's t I checked the code and didn't see any occurrence where tablet_id is going to be empty. Removed the check. http://gerrit.cloudera.org:8080/#/c/20270/12/src/kudu/client/meta_cache.cc@552 PS12, Line 552: remote_tablet > What if the range is now a non-covered one? The 'remote_tablet' will be a I am not sure if non-covered case will be applicable here. In this specific scenario, we will land here only when we have to pick a leader for a tablet replica in order to direct incoming op there. I tested some cases to induce PickLeader on a table with non-covered range by issuing insert but every time it hits lookup error and does an early exit without invoking pick leader that would eventually end up here. I can add a nullptr check just to be sure. -- To view, visit http://gerrit.cloudera.org:8080/20270 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia09cf6fb1b1d10f1ad13a62b5c863bcd1e3ab26a Gerrit-Change-Number: 20270 Gerrit-PatchSet: 12 Gerrit-Owner: Ashwani Raina <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Comment-Date: Mon, 11 Sep 2023 15:01:34 +0000 Gerrit-HasComments: Yes
