Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-5538: Use explicit catalog versions for deleted objects ......................................................................
Patch Set 5: (2 comments) Addressing some questions by Dan. http://gerrit.cloudera.org:8080/#/c/7731/5/common/thrift/CatalogInternalService.thrift File common/thrift/CatalogInternalService.thrift: PS5, Line 38: empty list if no objects detected in the Catalog > s/through/trouble/g Yes, there is some context that is missing here, hence it's not easy to understand what these comments mean. Let me try to explain first here and I will expand the comments to clarify some things. 1. What I think that comment should say is "no deleted objects were detected since the last GetAllCatalogObjects request". 2. Similarly to the previous comment. Deleted and updated/new objects are with respect to the previous GetAllCatalogObjects call. The base from which these deltas are computed is the "fromVersion" param of the TGetAllCatalogObjects struct. I'll update the comments. http://gerrit.cloudera.org:8080/#/c/7731/5/common/thrift/StatestoreService.thrift File common/thrift/StatestoreService.thrift: Line 84: // is not included in the topic key (e.g. catalog version of deleted catalog objects). > Sorry, opaque was the wrong word. I meant to say agnostic. Let me try to explain. Statestore only uses that flag to avoid sending deleted topic entries when the topic update is not a delta one (this is an optimization). The statestore subscribers (e.g. impalad) are the ones that really need what's in the 'value' field in order to determine how to process a deleted topic entry. For the case of catalog update, the value contains among other things the catalog version when the deletion occurred and the impalad uses that information to determine if the deletion should be applied at the local catalog or not. We can definitely add the deleted flag inside whatever object we're putting in 'value' (e.g. TCatalogObject) and I am happy to try this out if you think it's better. -- To view, visit http://gerrit.cloudera.org:8080/7731 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I93cb7a033dc8f0d3e0339394b36affe14523274c Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-HasComments: Yes
