[IMPALA-8312] Alter database operations have race condition

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Fix Version/s: Impala 3.3.0
• Epic Color: ghx-label-4
• Assignee: Vihang Karajgaonkar
• Reporter: Vihang Karajgaonkar
• Created: 14/Mar/19 20:44
• Updated: 09/Apr/19 19:48
• Resolved: 09/Apr/19 19:48

Description

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.

  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.");
  }

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

  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.");
  }

Issue Links

• links to: gerrit_review