impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5538: Use explicit catalog versions for deleted objects
Date Thu, 14 Sep 2017 22:21:48 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5538: Use explicit catalog versions for deleted objects
......................................................................


Patch Set 5:

(2 comments)

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
> Sorry not following. This says detected not deleted. Maybe I misunderstood 
I wasn't very clear. I was making two separate points:
1) The parenthesis statement makes it sound like the (only) condition where this list is empty
is when no objects are detected in the catalog, but it seems that this list can also be empty
if nothing is deleted.

2) Separately (and not related to the highlighted code so sorry for the confusion), I'm having
through understanding what 'update_objects' and 'deleted_objects' really mean since those
are ways to express things as a delta from some list, but I don't know what that list is.
In the old structure, 'objects' was just an absolute thing (not relative to a previous list).


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).
> Hm, I don't see why the value should be opaque to the subscriber. On the co
Sorry, opaque was the wrong word. I meant to say agnostic.

I agree value should be opaque to statestore. What I don't understand is how 'delete' can
have meaning at the statestore level if a subscriber needs to look inside 'value' to know
how to apply it.  So, is 'deleted' opaque to the statestore, and if so, why not just put that
in the 'value'?  If statestore does use 'deleted' for something, how can that be given that
'value' contains information needed in order to apply the delete?


-- 
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