This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 43caa56e18 HDDS-7178. [Multi-Tenant] Use optimistic read in Ranger
background sync (#3725)
43caa56e18 is described below
commit 43caa56e18c4654cd3b58452152b14f3056c0376
Author: Ethan Rose <[email protected]>
AuthorDate: Fri Feb 17 08:51:27 2023 -0800
HDDS-7178. [Multi-Tenant] Use optimistic read in Ranger background sync
(#3725)
---
.../ozone/om/multitenant/AuthorizerLock.java | 15 +++++--
.../ozone/om/multitenant/AuthorizerLockImpl.java | 24 +++++++++--
.../ozone/om/service/OMRangerBGSyncService.java | 46 ++++++++++++++--------
.../hadoop/ozone/om/TestAuthorizerLockImpl.java | 37 ++++++++++++++++-
4 files changed, 97 insertions(+), 25 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLock.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLock.java
index 448e9d789a..1dcb0f0cd6 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLock.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLock.java
@@ -42,10 +42,19 @@ public interface AuthorizerLock {
void unlockRead(long stamp);
/**
- * A wrapper around tryReadLock() that throws when timed out.
- * @return stamp
+ * @return stamp that can be passed to
+ * {@link this#validateOptimisticRead(long)} to check if a write lock was
+ * acquired since the stamp was issued.
+ * @throws IOException If an ongoing write prevents the lock from moving to
+ * the read state for longer than the timeout.
+ */
+ long tryOptimisticReadThrowOnTimeout() throws IOException;
+
+ /**
+ * @return True if the write lock was not acquired since this stamp was
+ * issued for an optimistic read. False otherwise.
*/
- long tryReadLockThrowOnTimeout() throws IOException;
+ boolean validateOptimisticRead(long stamp);
/**
* Attempt to acquire the write lock to authorizer with a timeout.
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLockImpl.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLockImpl.java
index f3ed2c3130..bffda3ed8f 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLockImpl.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/multitenant/AuthorizerLockImpl.java
@@ -68,7 +68,7 @@ public class AuthorizerLockImpl implements AuthorizerLock {
}
@Override
- public long tryReadLockThrowOnTimeout() throws IOException {
+ public long tryOptimisticReadThrowOnTimeout() throws IOException {
long stamp;
try {
@@ -80,11 +80,27 @@ public class AuthorizerLockImpl implements AuthorizerLock {
throw new OMException("Timed out acquiring authorizer read lock."
+ " Another multi-tenancy request is in-progress. Try again later",
ResultCodes.TIMEOUT);
+ }
+
+ long optimisticStamp = authorizerStampedLock.tryConvertToOptimisticRead(
+ stamp);
+ if (optimisticStamp == 0L) {
+ // This should never happen. If we reached this point we are holding a
+ // read lock and providing its stamp so the lock should be in read mode
+ // already. This would return a valid stamp, not 0.
+ unlockRead(stamp);
+ throw new OMException("Failed to convert read lock to optimistic read.",
+ INTERNAL_ERROR);
} else if (LOG.isDebugEnabled()) {
- LOG.debug("Acquired authorizer read lock from thread {} with stamp {}",
- Thread.currentThread().getId(), stamp);
+ LOG.debug("Acquired authorizer optimistic read from thread {} with" +
+ " stamp {}", Thread.currentThread().getId(), optimisticStamp);
}
- return stamp;
+ return optimisticStamp;
+ }
+
+ @Override
+ public boolean validateOptimisticRead(long stamp) {
+ return authorizerStampedLock.validate(stamp);
}
@Override
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OMRangerBGSyncService.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OMRangerBGSyncService.java
index 82e0d4972a..eb6f852217 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OMRangerBGSyncService.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/OMRangerBGSyncService.java
@@ -296,6 +296,7 @@ public class OMRangerBGSyncService extends
BackgroundService {
* @return true if completed successfully, false if any exception is thrown.
*/
public synchronized boolean triggerRangerSyncOnce() {
+ int attempt = 0;
try {
long dbOzoneServiceVersion = getOMDBRangerServiceVersion();
long rangerOzoneServiceVersion = getRangerOzoneServicePolicyVersion();
@@ -319,14 +320,11 @@ public class OMRangerBGSyncService extends
BackgroundService {
// A maximum of MAX_ATTEMPT times will be attempted each time the sync
// service is run. MAX_ATTEMPT should at least be 2 to make sure OM DB
// has the up-to-date Ranger service version most of the times.
- int attempt = 0;
while (dbOzoneServiceVersion != rangerOzoneServiceVersion) {
if (++attempt > MAX_ATTEMPT) {
- if (LOG.isDebugEnabled()) {
- LOG.warn("Reached maximum number of attempts ({}). Abort",
- MAX_ATTEMPT);
- }
+ LOG.warn("Reached maximum number of attempts ({}). Abort",
+ MAX_ATTEMPT);
break;
}
@@ -350,9 +348,12 @@ public class OMRangerBGSyncService extends
BackgroundService {
}
} catch (IOException | ServiceException e) {
LOG.warn("Exception during Ranger Sync", e);
- // TODO: Check for specific exception once switched to
- // RangerRestMultiTenantAccessController
return false;
+ } finally {
+ if (attempt > 0) {
+ LOG.info("Finished executing Multi-Tenancy Ranger Sync run # {} after"
+
+ "{} attempts.", runCount.get(), attempt);
+ }
}
return true;
@@ -424,7 +425,7 @@ public class OMRangerBGSyncService extends
BackgroundService {
clearPolicyAndRoleMaps();
- withReadLock(() -> {
+ withOptimisticRead(() -> {
try {
loadAllPoliciesAndRoleNamesFromRanger(baseVersion);
loadAllRolesFromRanger();
@@ -561,16 +562,29 @@ public class OMRangerBGSyncService extends
BackgroundService {
}
/**
- * Helper function to run the block with read lock held.
+ * Helper function to retry the block until it completes without a write lock
+ * being acquired during its execution. The block will be retried
+ * {@link this#MAX_ATTEMPT} times.
*/
- private void withReadLock(Runnable block) throws IOException {
- // Acquire authorizer (Ranger) read lock
- long stamp = authorizerLock.tryReadLockThrowOnTimeout();
- try {
+ private void withOptimisticRead(Runnable block) throws IOException {
+ // Acquire a stamp that will be used to check if a write occurred while we
+ // were reading.
+ // If a tenant modification is made while we are reading,
+ // retry the read operation with a new stamp until we are able to read the
+ // state without a write operation interrupting.
+ int attempt = 0;
+ boolean readSuccess = false;
+ while (!readSuccess && attempt < MAX_ATTEMPT) {
+ long stamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
block.run();
- } finally {
- // Release authorizer (Ranger) read lock
- authorizerLock.unlockRead(stamp);
+ readSuccess = authorizerLock.validateOptimisticRead(stamp);
+ attempt++;
+ }
+
+ if (!readSuccess) {
+ throw new IOException("Failed to read state for Ranger background sync" +
+ " without an interrupting write operation after " + attempt +
+ " attempts.");
}
}
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestAuthorizerLockImpl.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestAuthorizerLockImpl.java
index 502537cb2f..4d5850e872 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestAuthorizerLockImpl.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestAuthorizerLockImpl.java
@@ -132,14 +132,15 @@ public class TestAuthorizerLockImpl {
}
@Test
- public void testIsWriteLockHeldByCurrentThread() throws IOException {
+ public void testIsWriteLockHeldByCurrentThread() throws IOException,
+ InterruptedException {
final AuthorizerLock authorizerLock = new AuthorizerLockImpl();
Assert.assertFalse(authorizerLock.isWriteLockHeldByCurrentThread());
// Read lock does not affect the check
- long readLockStamp = authorizerLock.tryReadLockThrowOnTimeout();
+ long readLockStamp = authorizerLock.tryReadLock(100);
Assert.assertFalse(authorizerLock.isWriteLockHeldByCurrentThread());
authorizerLock.unlockRead(readLockStamp);
@@ -153,4 +154,36 @@ public class TestAuthorizerLockImpl {
Assert.assertFalse(authorizerLock.isWriteLockHeldByCurrentThread());
authorizerLock.unlockWrite(writeLockStamp);
}
+
+ @Test
+ public void testOptimisticRead() throws Exception {
+ final AuthorizerLock authorizerLock = new AuthorizerLockImpl();
+
+ // With no competing operations, an optimistic read should be valid.
+ long stamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
+ Assert.assertTrue(authorizerLock.validateOptimisticRead(stamp));
+
+ // With only competing reads, an optimistic read should be valid.
+ long optStamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
+ long readStamp = authorizerLock.tryReadLock(100);
+ Assert.assertTrue(readStamp > 0L);
+ Assert.assertTrue(authorizerLock.validateOptimisticRead(optStamp));
+ authorizerLock.unlockRead(readStamp);
+ Assert.assertTrue(authorizerLock.validateOptimisticRead(optStamp));
+
+ // When a write lock is held, optimistic read should time out trying to get
+ // a stamp.
+ long writeStamp = authorizerLock.tryWriteLockThrowOnTimeout();
+ Assert.assertThrows(IOException.class,
+ authorizerLock::tryOptimisticReadThrowOnTimeout);
+ authorizerLock.unlockWrite(writeStamp);
+
+ // When a write lock is acquired after the optimistic read stamp, the read
+ // stamp should be invalidated.
+ optStamp = authorizerLock.tryOptimisticReadThrowOnTimeout();
+ writeStamp = authorizerLock.tryWriteLockThrowOnTimeout();
+ Assert.assertTrue(writeStamp > 0L);
+ Assert.assertFalse(authorizerLock.validateOptimisticRead(optStamp));
+ authorizerLock.unlockWrite(writeStamp);
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
