[
https://issues.apache.org/jira/browse/SENTRY-1474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15523935#comment-15523935
]
Alex Leblang commented on SENTRY-1474:
--------------------------------------
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
>
> 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)
