You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sentry.apache.org by "Alex Leblang (JIRA)" <ji...@apache.org> on 2016/09/26 20:41:21 UTC

[jira] [Comment Edited] (SENTRY-1474) createSentryRole() isn't thread-safe

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

Alex Leblang edited comment on SENTRY-1474 at 9/26/16 8:40 PM:
---------------------------------------------------------------

I looked into this concern and wrote a test to attempt to replicate the suspected issue

In sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java I added a sleep to the createSentryRoleCore method and added a public dummy method for testing

{code:title=sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java|borderStyle=solid}
  private void createSentryRoleCore(PersistenceManager pm, String roleName, int sleepNum)
      throws SentryAlreadyExistsException {
    String trimmedRoleName = trimAndLower(roleName);
    MSentryRole mSentryRole = getMSentryRole(pm, trimmedRoleName);
    try {
      Thread.sleep(sleepNum);
    } catch (InterruptedException e) {
    }
    if (mSentryRole == null) {
      MSentryRole mRole = new MSentryRole(trimmedRoleName, System.currentTimeMillis());
      pm.makePersistent(mRole);
    } else {
      throw new SentryAlreadyExistsException("Role: " + trimmedRoleName);
    }
  }

  public void dummyCreateSentryRole(String roleName, CyclicBarrier barrier)
      throws SentryAlreadyExistsException, SentryStandbyException, InterruptedException,
      BrokenBarrierException {
    boolean rollbackTransaction = true;
    PersistenceManager pm = null;
    try {
      pm = openTransaction();
      act.checkSqlFencing(pm);
      barrier.await();
      createSentryRoleCore(pm, roleName, 2000);
      CommitContext commit = commitUpdateTransaction(pm);
      rollbackTransaction = false;
    } finally {
      if (rollbackTransaction) {
        rollbackTransaction(pm);
      }
    }
  }
{code}

I created a test to call the dummy method in sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
{code:title=sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java|borderStyle=solid}
  class createRoleWorker implements Runnable {
    CyclicBarrier barrier_;
    String roleName_;

    createRoleWorker(CyclicBarrier barrier, String roleName) {
      barrier_ = barrier;
      roleName_ = roleName;
     }

    public void run() {
      try {
        sentryStore.dummyCreateSentryRole(roleName_, barrier_);
      } catch (SentryAlreadyExistsException | SentryStandbyException
          | InterruptedException | BrokenBarrierException e) {
        e.printStackTrace();
      }
    }
  }

  @Test
  public void testCreateSentryRole() throws Exception {
    CyclicBarrier barrier = new CyclicBarrier(2);
    String roleName = "dummyRole";
    createRoleWorker firstWorker = new createRoleWorker(barrier, roleName);
    createRoleWorker secondWorker = new createRoleWorker(barrier, roleName);
    Thread one = new Thread(firstWorker);
    Thread two = new Thread(secondWorker);
    one.start();
    two.start();
    one.join();
    two.join();
  }
{code}

When looking for the race condition I got this exception:
{code} Exception in thread "Thread-4" javax.jdo.JDODataStoreException: Insert of object "org.apache.sentry.provider.db.service.model.MSentryRole@36c2b1d3" using statement "INSERT INTO SENTRY_ROLE (ROLE_ID,ROLE_NAME,CREATE_TIME) VALUES (?,?,?)" failed : The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.datanucleus.api.jdo.NucleusJDOHelper.getJDOExceptionForNucleusException(NucleusJDOHelper.java:451)
        at org.datanucleus.api.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:732)
        at org.datanucleus.api.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:752)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.createSentryRoleCore(SentryStore.java:362)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.dummyCreateSentryRole(SentryStore.java:378)
        at org.apache.sentry.provider.db.service.persistent.TestSentryStore$createRoleWorker.run(TestSentryStore.java:2125)
        at java.lang.Thread.run(Thread.java:745)
NestedThrowablesStackTrace:
java.sql.SQLIntegrityConstraintViolationException: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedConnection.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.ConnectionChild.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeStatement(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeLargeUpdate(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeUpdate(Unknown Source)
        at com.jolbox.bonecp.PreparedStatementHandle.executeUpdate(PreparedStatementHandle.java:203)
        at org.datanucleus.store.rdbms.ParamLoggingPreparedStatement.executeUpdate(ParamLoggingPreparedStatement.java:399)
        at org.datanucleus.store.rdbms.SQLController.executeStatementUpdate(SQLController.java:439)
        at org.datanucleus.store.rdbms.request.InsertRequest.execute(InsertRequest.java:410)
        at org.datanucleus.store.rdbms.RDBMSPersistenceHandler.insertTable(RDBMSPersistenceHandler.java:167)
        at org.datanucleus.store.rdbms.RDBMSPersistenceHandler.insertObject(RDBMSPersistenceHandler.java:143)
        at org.datanucleus.state.JDOStateManager.internalMakePersistent(JDOStateManager.java:3784)
        at org.datanucleus.state.JDOStateManager.makePersistent(JDOStateManager.java:3760)
        at org.datanucleus.ExecutionContextImpl.persistObjectInternal(ExecutionContextImpl.java:2219)
        at org.datanucleus.ExecutionContextImpl.persistObjectWork(ExecutionContextImpl.java:2065)
        at org.datanucleus.ExecutionContextImpl.persistObject(ExecutionContextImpl.java:1913)
        at org.datanucleus.ExecutionContextThreadedImpl.persistObject(ExecutionContextThreadedImpl.java:217)
        at org.datanucleus.api.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:727)
        at org.datanucleus.api.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:752)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.createSentryRoleCore(SentryStore.java:362)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.dummyCreateSentryRole(SentryStore.java:378)
        at org.apache.sentry.provider.db.service.persistent.TestSentryStore$createRoleWorker.run(TestSentryStore.java:2125)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.sql.SQLException: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(Unknown Source)
        ... 28 more
Caused by: ERROR 23505: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.iapi.error.StandardException.newException(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.insertAndCheckDups(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.doInsert(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.insert(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexSetChanger.insert(Unknown Source)
        at org.apache.derby.impl.sql.execute.RowChangerImpl.insertRow(Unknown Source)
        at org.apache.derby.impl.sql.execute.InsertResultSet.normalInsertCore(Unknown Source)
        at org.apache.derby.impl.sql.execute.InsertResultSet.open(Unknown Source)
        at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)
        at org.apache.derby.impl.sql.GenericPreparedStatement.execute(Unknown Source)
        ... 22 more
{code}

So, due to the primary key check, this is not a bug. We may revisit this issue if the primary key dependency is removed.


was (Author: awleblang):
I looked into this concern and wrote a test to attempt to replicate the suspected issue

In sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java I added a sleep to the createSentryRoleCore method and added a public dummy method for testing

{code:title=sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java|borderStyle=solid}
  private void createSentryRoleCore(PersistenceManager pm, String roleName, int sleepNum)
      throws SentryAlreadyExistsException {
    String trimmedRoleName = trimAndLower(roleName);
    MSentryRole mSentryRole = getMSentryRole(pm, trimmedRoleName);
    try {
      System.out.println("Above sleep");
      Thread.sleep(sleepNum);
      System.out.println("Below sleep");
    } catch (InterruptedException e) {
    }
    if (mSentryRole == null) {
      System.out.println("Entered key if");
      MSentryRole mRole = new MSentryRole(trimmedRoleName, System.currentTimeMillis());
      pm.makePersistent(mRole);
    } else {
      throw new SentryAlreadyExistsException("Role: " + trimmedRoleName);
    }
  }

  public void dummyCreateSentryRole(String roleName, CyclicBarrier barrier)
      throws SentryAlreadyExistsException, SentryStandbyException, InterruptedException,
      BrokenBarrierException {
    boolean rollbackTransaction = true;
    PersistenceManager pm = null;
    try {
      pm = openTransaction();
      act.checkSqlFencing(pm);
      barrier.await();
      createSentryRoleCore(pm, roleName, 2000);
      CommitContext commit = commitUpdateTransaction(pm);
      rollbackTransaction = false;
    } finally {
      if (rollbackTransaction) {
        rollbackTransaction(pm);
      }
    }
  }
{code}

I created a test to call the dummy method in sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java
{code:title=sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestSentryStore.java|borderStyle=solid}
  class createRoleWorker implements Runnable {
    CyclicBarrier barrier_;
    String roleName_;

    createRoleWorker(CyclicBarrier barrier, String roleName) {
      barrier_ = barrier;
      roleName_ = roleName;
     }

    public void run() {
      try {
        sentryStore.dummyCreateSentryRole(roleName_, barrier_);
      } catch (SentryAlreadyExistsException | SentryStandbyException
          | InterruptedException | BrokenBarrierException e) {
        e.printStackTrace();
      }
    }
  }

  @Test
  public void testCreateSentryRole() throws Exception {
    CyclicBarrier barrier = new CyclicBarrier(2);
    String roleName = "dummyRole";
    createRoleWorker firstWorker = new createRoleWorker(barrier, roleName);
    createRoleWorker secondWorker = new createRoleWorker(barrier, roleName);
    Thread one = new Thread(firstWorker);
    Thread two = new Thread(secondWorker);
    one.start();
    two.start();
    one.join();
    two.join();
  }
{code}

When looking for the race condition I got this exception:
{code} Exception in thread "Thread-4" javax.jdo.JDODataStoreException: Insert of object "org.apache.sentry.provider.db.service.model.MSentryRole@36c2b1d3" using statement "INSERT INTO SENTRY_ROLE (ROLE_ID,ROLE_NAME,CREATE_TIME) VALUES (?,?,?)" failed : The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.datanucleus.api.jdo.NucleusJDOHelper.getJDOExceptionForNucleusException(NucleusJDOHelper.java:451)
        at org.datanucleus.api.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:732)
        at org.datanucleus.api.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:752)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.createSentryRoleCore(SentryStore.java:362)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.dummyCreateSentryRole(SentryStore.java:378)
        at org.apache.sentry.provider.db.service.persistent.TestSentryStore$createRoleWorker.run(TestSentryStore.java:2125)
        at java.lang.Thread.run(Thread.java:745)
NestedThrowablesStackTrace:
java.sql.SQLIntegrityConstraintViolationException: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.getSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.Util.generateCsSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.wrapInSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.TransactionResourceImpl.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedConnection.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.ConnectionChild.handleException(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedStatement.executeStatement(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeStatement(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeLargeUpdate(Unknown Source)
        at org.apache.derby.impl.jdbc.EmbedPreparedStatement.executeUpdate(Unknown Source)
        at com.jolbox.bonecp.PreparedStatementHandle.executeUpdate(PreparedStatementHandle.java:203)
        at org.datanucleus.store.rdbms.ParamLoggingPreparedStatement.executeUpdate(ParamLoggingPreparedStatement.java:399)
        at org.datanucleus.store.rdbms.SQLController.executeStatementUpdate(SQLController.java:439)
        at org.datanucleus.store.rdbms.request.InsertRequest.execute(InsertRequest.java:410)
        at org.datanucleus.store.rdbms.RDBMSPersistenceHandler.insertTable(RDBMSPersistenceHandler.java:167)
        at org.datanucleus.store.rdbms.RDBMSPersistenceHandler.insertObject(RDBMSPersistenceHandler.java:143)
        at org.datanucleus.state.JDOStateManager.internalMakePersistent(JDOStateManager.java:3784)
        at org.datanucleus.state.JDOStateManager.makePersistent(JDOStateManager.java:3760)
        at org.datanucleus.ExecutionContextImpl.persistObjectInternal(ExecutionContextImpl.java:2219)
        at org.datanucleus.ExecutionContextImpl.persistObjectWork(ExecutionContextImpl.java:2065)
        at org.datanucleus.ExecutionContextImpl.persistObject(ExecutionContextImpl.java:1913)
        at org.datanucleus.ExecutionContextThreadedImpl.persistObject(ExecutionContextThreadedImpl.java:217)
        at org.datanucleus.api.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:727)
        at org.datanucleus.api.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:752)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.createSentryRoleCore(SentryStore.java:362)
        at org.apache.sentry.provider.db.service.persistent.SentryStore.dummyCreateSentryRole(SentryStore.java:378)
        at org.apache.sentry.provider.db.service.persistent.TestSentryStore$createRoleWorker.run(TestSentryStore.java:2125)
        at java.lang.Thread.run(Thread.java:745)
Caused by: java.sql.SQLException: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.impl.jdbc.SQLExceptionFactory.getSQLException(Unknown Source)
        at org.apache.derby.impl.jdbc.SQLExceptionFactory40.wrapArgsForTransportAcrossDRDA(Unknown Source)
        ... 28 more
Caused by: ERROR 23505: The statement was aborted because it would have caused a duplicate key value in a unique or primary key constraint or unique index identified by 'SENTRY_ROLE_NAME' defined on 'SENTRY_ROLE'.
        at org.apache.derby.iapi.error.StandardException.newException(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.insertAndCheckDups(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.doInsert(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexChanger.insert(Unknown Source)
        at org.apache.derby.impl.sql.execute.IndexSetChanger.insert(Unknown Source)
        at org.apache.derby.impl.sql.execute.RowChangerImpl.insertRow(Unknown Source)
        at org.apache.derby.impl.sql.execute.InsertResultSet.normalInsertCore(Unknown Source)
        at org.apache.derby.impl.sql.execute.InsertResultSet.open(Unknown Source)
        at org.apache.derby.impl.sql.GenericPreparedStatement.executeStmt(Unknown Source)
        at org.apache.derby.impl.sql.GenericPreparedStatement.execute(Unknown Source)
        ... 22 more
{code}

So, due to the primary key check, this is not a bug. We may revisit this issue if the primary key dependency is removed.

> createSentryRole() isn't thread-safe
> ------------------------------------
>
>                 Key: SENTRY-1474
>                 URL: https://issues.apache.org/jira/browse/SENTRY-1474
>             Project: Sentry
>          Issue Type: Bug
>          Components: Core
>    Affects Versions: 1.7.0, sentry-ha-redesign
>            Reporter: Alexander Kolbasov
>            Assignee: Alex Leblang
>
> Here is the function createSentryRole():
> {code}
>   public CommitContext createSentryRole(String roleName)
>       throws SentryAlreadyExistsException, SentryStandbyException {
>     boolean rollbackTransaction = true;
>     PersistenceManager pm = null;
>     try {
>       pm = openTransaction();
>       createSentryRoleCore(pm, roleName);
>       CommitContext commit = commitUpdateTransaction(pm);
>       rollbackTransaction = false;
>       return commit;
>     } finally {
>        ...
>     }
>   }
> {code}
> And here is createSentryRoleCore():
> {code}
>   private void createSentryRoleCore(PersistenceManager pm, String roleName)
>       throws SentryAlreadyExistsException {
>     String trimmedRoleName = trimAndLower(roleName);
>     MSentryRole mSentryRole = getMSentryRole(pm, trimmedRoleName);
>     if (mSentryRole == null) {
>       MSentryRole mRole = new MSentryRole(trimmedRoleName, System.currentTimeMillis());
>       pm.makePersistent(mRole);
>     } else {
>       throw new SentryAlreadyExistsException("Role: " + trimmedRoleName);
>     }
>   }
> {code}
> The problem is that after the call to getMSentryRole() the role can be added by another thread. So we will successfully add two instances of a role with the same name. After that calls to getMSentryRole() with this name will fail because we have two instances with the same name.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)