Huaisi Xu has posted comments on this change.

Change subject: IMPALA-3499: Batch update catalog cache update.
......................................................................


Patch Set 8:

(4 comments)

thanks for reviewing!

http://gerrit.cloudera.org:8080/#/c/3067/8/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

PS8, Line 124: A single object will not be limited by this.
> How can you guarantee this?
the logic from line 1296 to 1309 guarantees that no matter how large an object 
is, it will get a chance to be pushed to topic update. The check only batches 
update(and clear) anything that is currently in the vector.


Line 1271:     }
> I believe we should log it if L1265 does not return OK.
this will get logged at line 1282 anyway. (we are processing this single object 
twice). I think we do not change anything here. Previously it will fail at line 
1279, so frontend sees catalogID change and will request full update BEFORE any 
update. Now, it is the same thing, it will fail the same way either the first 
batch update or full unbatched update, again, BEFORE any update. I admit this 
corner case is a bit tricky... sorry


PS8, Line 1306: true
> How do you know that this is not part of an initial catalog update?
Because I already did update once at line 1303. the following update must not 
be is_delta otherwise the catalog cache will be cleared again.


PS8, Line 1323: We
              :     // need to do this before updating the catalog.
> This comment indicates that the modifications above may not be correct. I f
I think this means that we cannot delete those in the catalog cache before we 
do line 1342? that only checks whether we are deleting something newly 
created(has a higher version number)?


-- 
To view, visit http://gerrit.cloudera.org:8080/3067
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I176db25124a32944f2396ce8aafbed49cac95928
Gerrit-PatchSet: 8
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Huaisi Xu <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to