[
https://issues.apache.org/jira/browse/HDDS-1672?focusedWorklogId=264049&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-264049
]
ASF GitHub Bot logged work on HDDS-1672:
----------------------------------------
Author: ASF GitHub Bot
Created on: 20/Jun/19 19:12
Start Date: 20/Jun/19 19:12
Worklog Time Spent: 10m
Work Description: bharatviswa504 commented on pull request #949:
HDDS-1672. Improve locking in OzoneManager.
URL: https://github.com/apache/hadoop/pull/949#discussion_r295956276
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerLock.java
##########
@@ -154,79 +178,137 @@ public void releaseVolumeLock(String volume) {
}
/**
- * Acquires S3 Bucket lock on the given resource.
+ * Acquires bucket lock on the given resource.
*
* <p>If the lock is not available then the current thread becomes
- * disabled for thread scheduling purposes and lies dormant until the lock
has
- * been acquired.
+ * disabled for thread scheduling purposes and lies dormant until the
+ * lock has been acquired.
*
- * @param s3BucketName S3Bucket Name on which the lock has to be acquired
+ * @param bucket Bucket on which the lock has to be acquired
*/
- public void acquireS3Lock(String s3BucketName) {
- // Calling thread should not hold any bucket lock.
- // You can take an Volume while holding S3 bucket lock, since
- // semantically an S3 bucket maps to the ozone volume. So we check here
- // only if ozone bucket lock is taken.
- if (hasAnyBucketLock()) {
+ public void acquireBucketLock(String volume, String bucket) {
+ if (hasAnyUserLock()) {
throw new RuntimeException(
"Thread '" + Thread.currentThread().getName() +
- "' cannot acquire S3 bucket lock while holding Ozone bucket " +
- "lock(s).");
+ "' cannot acquire bucket lock while holding User lock.");
}
- manager.lock(OM_S3_PREFIX + s3BucketName);
- myLocks.get().get(S3_BUCKET_LOCK).incrementAndGet();
+ manager.lock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket);
+ myLocks.get().get(BUCKET_LOCK).incrementAndGet();
}
/**
- * Releases the volume lock on given resource.
+ * Releases the bucket lock on given resource.
*/
- public void releaseS3Lock(String s3BucketName) {
- manager.unlock(OM_S3_PREFIX + s3BucketName);
- myLocks.get().get(S3_BUCKET_LOCK).decrementAndGet();
+ public void releaseBucketLock(String volume, String bucket) {
+ manager.unlock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket);
+ myLocks.get().get(BUCKET_LOCK).decrementAndGet();
}
/**
- * Acquires bucket lock on the given resource.
+ * Acquires user lock on the given resource.
*
* <p>If the lock is not available then the current thread becomes
* disabled for thread scheduling purposes and lies dormant until the
* lock has been acquired.
*
- * @param bucket Bucket on which the lock has to be acquired
+ * @param user User on which the lock has to be acquired
*/
- public void acquireBucketLock(String volume, String bucket) {
- manager.lock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket);
- myLocks.get().get(BUCKET_LOCK).incrementAndGet();
+ public void acquireUserLock(String user) {
+ // In order to not maintain username's on which we have acquired lock,
+ // just checking have we acquired userLock before. If user want's to
+ // acquire user lock on multiple user's they should use
+ // acquireMultiUserLock. This is just a protection logic, to let not users
+ // use this if acquiring lock on multiple users. As currently, we have only
+ // use case we have for this is during setOwner operation in VolumeManager.
+ if (hasAnyUserLock()) {
+ LOG.error("Already have userLock");
+ throw new RuntimeException("For acquiring lock on multiple users, use " +
+ "acquireMultiLock method");
+ }
+ manager.lock(OM_USER_PREFIX + user);
+ myLocks.get().get(USER_LOCK).incrementAndGet();
}
/**
- * Releases the bucket lock on given resource.
+ * Releases the user lock on given resource.
*/
- public void releaseBucketLock(String volume, String bucket) {
- manager.unlock(OM_KEY_PREFIX + volume + OM_KEY_PREFIX + bucket);
- myLocks.get().get(BUCKET_LOCK).decrementAndGet();
+ public void releaseUserLock(String user) {
+ manager.unlock(OM_USER_PREFIX + user);
+ myLocks.get().get(USER_LOCK).decrementAndGet();
}
/**
- * Returns true if the current thread holds any volume lock.
- * @return true if current thread holds volume lock, else false
+ * Acquire user lock on 2 users. In this case, we compare 2 strings
+ * lexicographically, and acquire the locks according to the sorted order of
+ * the user names. In this way, when acquiring locks on multiple user's, we
+ * can avoid dead locks. This method should be called when single thread is
+ * acquiring lock on 2 users at a time.
+ *
+ * Example:
+ * ozone, hdfs -> lock acquire order will be hdfs, ozone
+ * hdfs, ozone -> lock acquire order will be hdfs, ozone
+ *
+ * @param newUser
+ * @param oldUser
*/
- private boolean hasAnyVolumeLock() {
- return myLocks.get().get(VOLUME_LOCK).get() != 0;
+ public void acquireMultiUserLock(String newUser, String oldUser) {
+ Preconditions.checkNotNull(newUser);
+ Preconditions.checkNotNull(oldUser);
+ int compare = newUser.compareTo(oldUser);
+
+ if (compare < 0) {
+ manager.lock(OM_USER_PREFIX + newUser);
Review comment:
Lock Object is re-entrant, there fore should not be an issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 264049)
Time Spent: 7h 10m (was: 7h)
> Improve locking in OzoneManager
> -------------------------------
>
> Key: HDDS-1672
> URL: https://issues.apache.org/jira/browse/HDDS-1672
> Project: Hadoop Distributed Data Store
> Issue Type: Improvement
> Components: Ozone Manager
> Affects Versions: 0.4.0
> Reporter: Bharat Viswanadham
> Assignee: Bharat Viswanadham
> Priority: Major
> Labels: pull-request-available
> Attachments: Ozone Locks in OM.pdf
>
> Time Spent: 7h 10m
> Remaining Estimate: 0h
>
> In this Jira, we shall follow the new lock ordering. In this way, in volume
> requests we can solve the issue of acquire/release/reacquire problem. And few
> bugs in the current implementation of S3Bucket/Volume operations.
>
> Currently after acquiring volume lock, we cannot acquire user lock.
> This is causing an issue in Volume request implementation,
> acquire/release/reacquire volume lock.
>
> Case of Delete Volume Request:
> # Acquire volume lock.
> # Get Volume Info from DB
> # Release Volume lock. (We are releasing the lock, because while acquiring
> volume lock, we cannot acquire user lock0
> # Get owner from volume Info read from DB
> # Acquire owner lock
> # Acquire volume lock
> # Do delete logic
> # release volume lock
> # release user lock
>
> We can avoid this acquire/release/reacquire lock issue by making volume lock
> as low weight.
>
> In this way, the above deleteVolume request will change as below
> # Acquire volume lock
> # Get Volume Info from DB
> # Get owner from volume Info read from DB
> # Acquire owner lock
> # Do delete logic
> # release owner lock
> # release volume lock.
> Same issue is seen with SetOwner for Volume request also.
> During HDDS-1620 [~arp] brought up this issue.
> I am proposing the above solution to solve this issue. Any other
> idea/suggestions are welcome.
> This also resolves a bug in setOwner for Volume request.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
