You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "vtutrinov (via GitHub)" <gi...@apache.org> on 2023/10/06 06:57:27 UTC
[PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
vtutrinov opened a new pull request, #5398:
URL: https://github.com/apache/ozone/pull/5398
## What changes were proposed in this pull request?
The OM's org.apache.hadoop.ozone.om.OzoneManagerServiceGrpc#submitRequest method uses UUID.randomUUID() (through org.apache.hadoop.ipc.ClientId#getClientId) to set the clientId for an omRequest. The latter has a synchronized block inside that leads to high lock contention in case of a big amount of tiny files (1K) read requests (we were testing a read of 15mln 1KiB files from 3 clients in 50 threads, 150 threads in total). Hence, the clientId generator should be thread-local
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9379
## How was this patch tested?
Manual test of reading of 15mln 1KiB files
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #5398:
URL: https://github.com/apache/ozone/pull/5398#issuecomment-1842812116
> @vtutrinov , thanks for working on this which is a very good performance improvement!
>
> Indeed, we could generate the `byte[]` directly instead of generating an `UUID` and then convert it to `byte[]`. We could also get ride of the `java-uuid-generator` dependency.
>
> ```java
> +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/UUIDUtil.java
> @@ -15,13 +15,9 @@
> * See the License for the specific language governing permissions and
> * limitations under the License.
> */
> -
> package org.apache.hadoop.util;
>
> -import com.fasterxml.uuid.Generators;
> -
> import java.security.SecureRandom;
> -import java.util.UUID;
>
> /**
> * Helper methods to deal with random UUIDs.
> @@ -35,8 +31,14 @@ private UUIDUtil() {
> private static final ThreadLocal<SecureRandom> GENERATOR =
> ThreadLocal.withInitial(SecureRandom::new);
>
> - public static UUID randomUUID() {
> - return Generators.randomBasedGenerator(GENERATOR.get()).generate();
> + public static byte[] randomUuidBytes() {
> + final byte[] bytes = new byte[16];
> + GENERATOR.get().nextBytes(bytes);
> + // See RFC 4122 section 4.4
> + bytes[6] &= 0x0f;
> + bytes[6] |= 0x40;
> + bytes[8] &= 0x3f;
> + bytes[8] |= 0x80;
> + return bytes;
> }
> -
> }
> ```
@szetszwo
https://github.com/apache/ozone/pull/5740
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5398:
URL: https://github.com/apache/ozone/pull/5398
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5398:
URL: https://github.com/apache/ozone/pull/5398#issuecomment-1756006658
Thanks @vtutrinov for the contribution.
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #5398:
URL: https://github.com/apache/ozone/pull/5398#issuecomment-1841711841
@vtutrinov , thanks for working on this which is a very good performance improvement!
Indeed, we could generate the `byte[]` directly instead of generating an `UUID` and then convert it to `byte[]`. We could also get ride of the `java-uuid-generator` dependency.
```java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/util/UUIDUtil.java
@@ -15,13 +15,9 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
package org.apache.hadoop.util;
-import com.fasterxml.uuid.Generators;
-
import java.security.SecureRandom;
-import java.util.UUID;
/**
* Helper methods to deal with random UUIDs.
@@ -35,8 +31,14 @@ private UUIDUtil() {
private static final ThreadLocal<SecureRandom> GENERATOR =
ThreadLocal.withInitial(SecureRandom::new);
- public static UUID randomUUID() {
- return Generators.randomBasedGenerator(GENERATOR.get()).generate();
+ public static byte[] randomUuidBytes() {
+ final byte[] bytes = new byte[16];
+ GENERATOR.get().nextBytes(bytes);
+ // See RFC 4122 section 4.4
+ bytes[6] &= 0x0f;
+ bytes[6] |= 0x40;
+ bytes[8] &= 0x3f;
+ bytes[8] |= 0x80;
+ return bytes;
}
-
}
```
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on code in PR #5398:
URL: https://github.com/apache/ozone/pull/5398#discussion_r1348648989
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java:
##########
@@ -96,4 +99,23 @@ public void submitRequest(OMRequest request,
}
responseObserver.onCompleted();
}
+
+ private static byte[] getClientId() {
+ UUID uuid = UUIDs.randomUUID();
+ ByteBuffer buf = ByteBuffer.wrap(new byte[16]);
+ buf.putLong(uuid.getMostSignificantBits());
+ buf.putLong(uuid.getLeastSignificantBits());
+ return buf.array();
+ }
+
+ private static class UUIDs {
+ private static final ThreadLocal<SecureRandom> GENERATOR =
+ ThreadLocal.withInitial(SecureRandom::new);
+
+ public static UUID randomUUID() {
+ return Generators.randomBasedGenerator(GENERATOR.get()).generate();
+ }
+
+ }
Review Comment:
moved to hadoop-hdds/common
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5398:
URL: https://github.com/apache/ozone/pull/5398#discussion_r1348574246
##########
pom.xml:
##########
@@ -309,6 +309,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
<jgrapht.version>1.0.1</jgrapht.version>
<vault.driver.version>5.1.0</vault.driver.version>
+ <java.uuid.generator.version>3.1.4</java.uuid.generator.version>
Review Comment:
Can you please explain how this specific version was selected?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManagerServiceGrpc.java:
##########
@@ -96,4 +99,23 @@ public void submitRequest(OMRequest request,
}
responseObserver.onCompleted();
}
+
+ private static byte[] getClientId() {
+ UUID uuid = UUIDs.randomUUID();
+ ByteBuffer buf = ByteBuffer.wrap(new byte[16]);
+ buf.putLong(uuid.getMostSignificantBits());
+ buf.putLong(uuid.getLeastSignificantBits());
+ return buf.array();
+ }
+
+ private static class UUIDs {
+ private static final ThreadLocal<SecureRandom> GENERATOR =
+ ThreadLocal.withInitial(SecureRandom::new);
+
+ public static UUID randomUUID() {
+ return Generators.randomBasedGenerator(GENERATOR.get()).generate();
+ }
+
+ }
Review Comment:
Shouldn't it be a top-level utility class in `hdds-common`? There are quite a few other usages of `UUID.randomUUID()` that may benefit from this.
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #5398:
URL: https://github.com/apache/ozone/pull/5398#issuecomment-1755189214
> Thanks @vtutrinov for the patch.
>
> > Manual test of reading of 15mln 1KiB files
>
> Can you please share some numbers about the improvement?
~ +17% to performance (previous result = 23k files/sec, new result = 27k files/sec)
--
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
Re: [PR] HDDS-9379. thread local client ID generator for omRequest [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on code in PR #5398:
URL: https://github.com/apache/ozone/pull/5398#discussion_r1348649740
##########
pom.xml:
##########
@@ -309,6 +309,7 @@ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xs
<jgrapht.version>1.0.1</jgrapht.version>
<vault.driver.version>5.1.0</vault.driver.version>
+ <java.uuid.generator.version>3.1.4</java.uuid.generator.version>
Review Comment:
as the first suggestion in IDE, changed to 4.3.0 (the last released version from the Maven Central)
--
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