[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4020: Catalog update can fail due to database
creation/deletion in hive
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4161/2/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:
PS2, Line 542: invalidateDatabase
You can make the function return a Pair and avoid passing
newDbCache and tblsToBackgroundLoad since these are output parameters.
PS2, Line 608: try {
You can actually push the try catch in that function and have it return null if
an error occurred.
--
To view, visit http://gerrit.cloudera.org:8080/4161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: anujphadke
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
anujphadke has posted comments on this change. Change subject: IMPALA-4020: Catalog update can fail due to database creation/deletion in hive .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/4161/1/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java: PS1, Line 590: "Error updating authorization policy: ", e); > What will happen if the database is dropped sometime before L588 and L590? I moved it to a function. Not sure if add those many parameters to a function is a good idea if we plan to reuse it somewhere else. I have moved newDbCache.put(db.getName().toLowerCase(), db); to the end like we discussed. Line 601: > You might want to add a comment here on when we catch this exception. Done Line 602: // Build a new DB cache, populate it, and replace the existing cache in one > IMO, this should be a WARN of lesser, given the DB has probably been delete Done -- To view, visit http://gerrit.cloudera.org:8080/4161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: anujphadke Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
anujphadke has uploaded a new patch set (#2). Change subject: IMPALA-4020: Catalog update can fail due to database creation/deletion in hive .. IMPALA-4020: Catalog update can fail due to database creation/deletion in hive Catalog currently makes more than one call to the hive metastore while invalidating a database. If a database gets dropped through hive while catalog metastore client tries to access this database a NoSuchObjectException is thrown. This stops initialzing the catalog object. This change catches the NoSuchObjectException and wont stop initializing the catalog object if this exception is thrown for any database. Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a --- M fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java 1 file changed, 49 insertions(+), 29 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/4161/2 -- To view, visit http://gerrit.cloudera.org:8080/4161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Dimitris Tsirogiannis
[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
Dimitris Tsirogiannis has posted comments on this change.
Change subject: IMPALA-4020: Catalog update can fail due to database
creation/deletion in hive
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4161/1/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:
PS1, Line 568: try {
indentation if off by 2 :)
PS1, Line 590: msClient.getHiveClient().getAllTables(dbName)
What will happen if the database is dropped sometime before L588 and L590? I
think we will get into this state where the database that we know got dropped
is added to the catalog. I understand that we can't really ensure atomicity
here but I think we should try to avoid creating inconsistent states as much as
possible.
First of all, I would extract the logic of invalidating the database into a
separate private function. Then I would check if that 'invalidateDb' finished
successfully or threw the exception and in the latter case, I would clean up
the catalog cache (if an entry was added). We don't have to worry about
concurrent operations because this function already has a write lock on the
catalog.
--
To view, visit http://gerrit.cloudera.org:8080/4161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
Bharath Vissapragada has posted comments on this change.
Change subject: IMPALA-4020: Catalog update can fail due to database
creation/deletion in hive
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/4161/1/fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java
File fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java:
Line 568: try {
Indentation off
Line 601: catch (NoSuchObjectException e) {
You might want to add a comment here on when we catch this exception.
Line 602: LOG.error("Error loading metadata for" + dbName);
IMO, this should be a WARN of lesser, given the DB has probably been deleted
outside of Impala and is a genuine case. Also may be format it to
LOG.warn("Encountered an exception while invalidating metadata for database: "
+ dbName, e) ? (just to signify its not really an error and also log the stack
trace for better debugging.)
--
To view, visit http://gerrit.cloudera.org:8080/4161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: anujphadke
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4020: Catalog update can fail due to database creation/deletion in hive
anujphadke has uploaded a new change for review. http://gerrit.cloudera.org:8080/4161 Change subject: IMPALA-4020: Catalog update can fail due to database creation/deletion in hive .. IMPALA-4020: Catalog update can fail due to database creation/deletion in hive Catalog currently makes more than one call to the hive metastore while invalidating a database. If a database gets dropped through hive while catalog metastore client tries to access this database a NoSuchObjectException is thrown. This stops initialzing the catalog object. This change catches the NoSuchObjectException and wont stop initializing the catalog object if this exception is thrown for any database. Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a --- M fe/src/main/java/com/cloudera/impala/catalog/CatalogServiceCatalog.java 1 file changed, 32 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/4161/1 -- To view, visit http://gerrit.cloudera.org:8080/4161 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ic228efbcceb9ef6c165d0d9aeef7202581e3e46a Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: anujphadke
