Henry Robinson has posted comments on this change. Change subject: IMPALA-5056: Ensure analysis uses 'fresh' catalog after metadata loading ......................................................................
Patch Set 4: (2 comments) Will post a new patch when I'm sure I'm addressing Alex's concerns. http://gerrit.cloudera.org:8080/#/c/7045/4/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: Line 610: List<String> tblNames = impaladCatalog_.get().getTableNames(dbName, matcher); > I'm a little worried about the repercussions of operations seeing different Can you clarify - is this an issue that you see introduced by this patch? getDbsMetadata() retrieves the catalog reference once. So it should see a consistent snapshot throughout its execution (or at least as consistent as it did previously). Previously, the naked catalog reference would be updated asynchronously. The only change here is an atomic ref that makes sure the update is published cross-thread. I agree that this is all a bit confusing. We should be able to snapshot a version of the catalog and keep it fixed during analysis. Line 890: AnalysisContext analysisCtx = new AnalysisContext(impaladCatalog_.get(), queryCtx, > Are we guaranteed that this new catalog returns true for isReady()? I believe so (since the new catalog is created from the statestore update). I'll look further and make the code clearer. -- To view, visit http://gerrit.cloudera.org:8080/7045 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I574d69fa75198499523dc291fbbd0d7e3d8d968f Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-HasComments: Yes