impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5538: Use explicit catalog versions for deleted objects
Date Thu, 14 Sep 2017 23:04:19 GMT
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 <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <vercegovac@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message