Alexey Serbin 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: (1 comment) 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) { : > DoFastPathLookup is a private method inside MetaCache and we are calling th Ah, I see: I didn't notice the call site of the new method was in MetaCacheServerPicker, not in the MetaCache class itself. Then the natural question is why to pollute MetaCacheServerPicker class with the logic that deals with possible inconsistencies in the MetaCache? Why not to move the newly introduced logic into the implementation of MetaCache::LookupTabletByKey()? Wouldn't it be more robust code then (BTW, the MetaCache::LookupTabletByKey() is used in several other places in the codebase)? -- 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: Tue, 29 Aug 2023 18:28:07 +0000 Gerrit-HasComments: Yes
