You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/04 03:28:28 UTC
[GitHub] [ozone] jojochuang opened a new pull request, #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
jojochuang opened a new pull request, #3654:
URL: https://github.com/apache/ozone/pull/3654
## What changes were proposed in this pull request?
Replace the spinlock in ChunkUtils with exponentially backoff timed wait.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-4970
## How was this patch tested?
The patch is applied and verified to work in a 20-node test cluster running Impala TPC-DS workload.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a diff in pull request #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3654:
URL: https://github.com/apache/ozone/pull/3654#discussion_r937460808
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java:
##########
@@ -291,10 +301,18 @@ public static void verifyChunkFileExists(File file)
}
@VisibleForTesting
- static <T> T processFileExclusively(Path path, Supplier<T> op) {
+ static <T> T processFileExclusively(Path path, Supplier<T> op)
+ throws InterruptedException {
+ long period = 1;
for (;;) {
if (LOCKS.add(path)) {
break;
+ } else {
+ Thread.sleep(period);
+ // exponentially backoff until the sleep time is over 1 second.
+ if (period < 1000) {
+ period *= 2;
Review Comment:
Got it, thanks for the clarification.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on PR #3654:
URL: https://github.com/apache/ozone/pull/3654#issuecomment-1206104546
Thanks @jojochuang for the patch, @symious for the review.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on a diff in pull request #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on code in PR #3654:
URL: https://github.com/apache/ozone/pull/3654#discussion_r937448224
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java:
##########
@@ -291,10 +301,18 @@ public static void verifyChunkFileExists(File file)
}
@VisibleForTesting
- static <T> T processFileExclusively(Path path, Supplier<T> op) {
+ static <T> T processFileExclusively(Path path, Supplier<T> op)
+ throws InterruptedException {
+ long period = 1;
for (;;) {
if (LOCKS.add(path)) {
break;
+ } else {
+ Thread.sleep(period);
+ // exponentially backoff until the sleep time is over 1 second.
+ if (period < 1000) {
+ period *= 2;
Review Comment:
I don't think it's necessary. `period` is not used after the lock is acquired. Other callers will start from 1, since it's a local variable.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
Posted by GitBox <gi...@apache.org>.
adoroszlai merged PR #3654:
URL: https://github.com/apache/ozone/pull/3654
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] symious commented on a diff in pull request #3654: HDDS-4970. Significant overhead when DataNode is over-subscribed
Posted by GitBox <gi...@apache.org>.
symious commented on code in PR #3654:
URL: https://github.com/apache/ozone/pull/3654#discussion_r937444904
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/helpers/ChunkUtils.java:
##########
@@ -291,10 +301,18 @@ public static void verifyChunkFileExists(File file)
}
@VisibleForTesting
- static <T> T processFileExclusively(Path path, Supplier<T> op) {
+ static <T> T processFileExclusively(Path path, Supplier<T> op)
+ throws InterruptedException {
+ long period = 1;
for (;;) {
if (LOCKS.add(path)) {
break;
+ } else {
+ Thread.sleep(period);
+ // exponentially backoff until the sleep time is over 1 second.
+ if (period < 1000) {
+ period *= 2;
Review Comment:
Should we reset the period when the lock is acquired?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org