[jira] [Commented] (IMPALA-8312) Alter database operations have race condition

["ASF subversion and git services (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IMPALA-8312?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16806686#comment-16806686 ] 

ASF subversion and git services commented on IMPALA-8312:
---------------------------------------------------------

Commit b2a87797a8be076a8e57a91e8db2692ca693e2f3 in impala's branch refs/heads/master from Vihang Karajgaonkar
[ https://gitbox.apache.org/repos/asf?p=impala.git;h=b2a8779 ]

IMPALA-8312 : Alter database operations have race condition

This patch fixes a race condition in the alter database implementation
in the catalogOpExecutor. The original implementation did a in-place
modification of the metastore database object in the Db. This can lead
to partially updated database object becoming visible to a reading
thread causing potential problems. In order to fix the race, the
patch makes a copy of the existing database object, modifies the copy
and then atomically switches the actual database object with the
modified copy. This is done while holding the metastoreddlLock, and
then taking the write lock on the catalog version object which makes
it consistent with the other catalog write operations.

Added a test which consistently reproduces the race. The test creating
many reader threads and a writer thread which continuously keeps
changing the owner name and its type by issuing a alter database
operation. The test fails without the patch. After the patch the test
passes. The race also applies to the alter database set comment
operation, although its hard to write a test for that code-path.

Change-Id: I32c8c96a6029bf9d9db37ea8315f6c9603b5a2fc
Reviewed-on: http://gerrit.cloudera.org:8080/12789
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


> Alter database operations have race condition
> ---------------------------------------------
>
>                 Key: IMPALA-8312
>                 URL: https://issues.apache.org/jira/browse/IMPALA-8312
>             Project: IMPALA
>          Issue Type: Bug
>            Reporter: Vihang Karajgaonkar
>            Assignee: Vihang Karajgaonkar
>            Priority: Major
>
> There are currently two {{alter database}} operations seen in {{CatalogOpExecutor}} although I only see one of them documented which is a separate issue.
> Consider the following snippet for {{alter database set owner}} operation.
> {code}
>   private void alterDatabaseSetOwner(String dbName, TAlterDbSetOwnerParams params,
>       TDdlExecResponse response) throws ImpalaException {
>     Db db = catalog_.getDb(dbName);
>     if (db == null) {
>       throw new CatalogException("Database: " + dbName + " does not exist.");
>     }
>     Preconditions.checkNotNull(params.owner_name);
>     Preconditions.checkNotNull(params.owner_type);
>     synchronized (metastoreDdlLock_) {
>       Database msDb = db.getMetaStoreDb();
>       String originalOwnerName = msDb.getOwnerName();
>       PrincipalType originalOwnerType = msDb.getOwnerType();
>       msDb.setOwnerName(params.owner_name);
>       msDb.setOwnerType(PrincipalType.valueOf(params.owner_type.name()));
>       try {
>         applyAlterDatabase(db);
>       } catch (ImpalaRuntimeException e) {
>         msDb.setOwnerName(originalOwnerName);
>         msDb.setOwnerType(originalOwnerType);
>         throw e;
>       }
>       updateOwnerPrivileges(db.getName(), /* tableName */ null, params.server_name,
>           originalOwnerName, originalOwnerType, db.getMetaStoreDb().getOwnerName(),
>           db.getMetaStoreDb().getOwnerType(), response);
>     }
>     addDbToCatalogUpdate(db, response.result);
>     addSummary(response, "Updated database.");
>   }
> {code}
> If you notice above, it takes a lock on {{metastoreDdlLock_}} but does not take a write lock on catalogVersion before altering the metastore db object in-place. This can lead to race conditions between a reader and writer thread. For example, it is possible that the thread which is reading {{msDb}} object can see new value of owner but a old value of owner type.
> The same problem applies to the alterCommentOnDb method below
> {code}
>   private void alterCommentOnDb(String dbName, String comment, TDdlExecResponse response)
>       throws ImpalaRuntimeException, CatalogException {
>     Db db = catalog_.getDb(dbName);
>     if (db == null) {
>       throw new CatalogException("Database: " + dbName + " does not exist.");
>     }
>     synchronized (metastoreDdlLock_) {
>       Database msDb = db.getMetaStoreDb();
>       String originalComment = msDb.getDescription();
>       msDb.setDescription(comment);
>       try {
>         applyAlterDatabase(db);
>       } catch (ImpalaRuntimeException e) {
>         msDb.setDescription(originalComment);
>         throw e;
>       }
>     }
>     addDbToCatalogUpdate(db, response.result);
>     addSummary(response, "Updated database.");
>   }
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org