This is an automated email from the ASF dual-hosted git repository.
szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git
The following commit(s) were added to refs/heads/master by this push:
new 639f1bf0b RATIS-1910. Deduplicate RaftGroupId and ClientId. (#940)
639f1bf0b is described below
commit 639f1bf0bc1d4b8c72e22cfcd50f3f1d2b7df641
Author: Tsz-Wo Nicholas Sze <[email protected]>
AuthorDate: Wed Oct 18 09:59:52 2023 -0700
RATIS-1910. Deduplicate RaftGroupId and ClientId. (#940)
---
.../java/org/apache/ratis/protocol/ClientId.java | 24 +++++----
.../org/apache/ratis/protocol/RaftGroupId.java | 23 ++++----
.../java/org/apache/ratis/protocol/RaftId.java | 61 +++++++++++++++++-----
.../java/org/apache/ratis/protocol/RaftPeerId.java | 9 ++--
.../com/google/protobuf/ByteStringUtils.java | 30 +++++++++++
.../java/org/apache/ratis/protocol/TestRaftId.java | 21 +++++++-
6 files changed, 129 insertions(+), 39 deletions(-)
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/ClientId.java
b/ratis-common/src/main/java/org/apache/ratis/protocol/ClientId.java
index 8c3e4c714..82ae426d5 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/ClientId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/ClientId.java
@@ -19,34 +19,38 @@ package org.apache.ratis.protocol;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
-import java.util.Optional;
import java.util.UUID;
/**
- * Id of Raft client. Should be globally unique so that raft peers can use it
+ * The id of RaftClient. Should be globally unique so that raft peers can use
it
* to correctly identify retry requests from the same client.
*/
public final class ClientId extends RaftId {
- private static final ClientId EMPTY_CLIENT_ID = new
ClientId(ZERO_UUID_BYTESTRING);
+ private static final Factory<ClientId> FACTORY = new Factory<ClientId>() {
+ @Override
+ ClientId newInstance(UUID uuid, ByteString bytes) {
+ return bytes == null? new ClientId(uuid): new ClientId(uuid, bytes);
+ }
+ };
public static ClientId emptyClientId() {
- return EMPTY_CLIENT_ID;
+ return FACTORY.emptyId();
}
public static ClientId randomId() {
- return new ClientId(UUID.randomUUID());
+ return FACTORY.randomId();
}
- public static ClientId valueOf(ByteString data) {
- return Optional.ofNullable(data).filter(d ->
!d.isEmpty()).map(ClientId::new).orElse(EMPTY_CLIENT_ID);
+ public static ClientId valueOf(ByteString bytes) {
+ return FACTORY.valueOf(bytes);
}
public static ClientId valueOf(UUID uuid) {
- return new ClientId(uuid);
+ return FACTORY.valueOf(uuid);
}
- private ClientId(ByteString data) {
- super(data);
+ private ClientId(UUID uuid, ByteString bytes) {
+ super(uuid, bytes);
}
private ClientId(UUID uuid) {
diff --git
a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
index f4820d614..968b9e500 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftGroupId.java
@@ -23,34 +23,39 @@ import java.util.UUID;
/**
* The id of a raft group.
- *
+ * <p>
* This is a value-based class.
*/
public final class RaftGroupId extends RaftId {
- private static final RaftGroupId EMPTY_GROUP_ID = new
RaftGroupId(ZERO_UUID_BYTESTRING);
+ private static final Factory<RaftGroupId> FACTORY = new
Factory<RaftGroupId>() {
+ @Override
+ RaftGroupId newInstance(UUID uuid, ByteString bytes) {
+ return bytes == null? new RaftGroupId(uuid) : new RaftGroupId(uuid,
bytes);
+ }
+ };
public static RaftGroupId emptyGroupId() {
- return EMPTY_GROUP_ID;
+ return FACTORY.emptyId();
}
public static RaftGroupId randomId() {
- return new RaftGroupId(UUID.randomUUID());
+ return FACTORY.randomId();
}
public static RaftGroupId valueOf(UUID uuid) {
- return new RaftGroupId(uuid);
+ return FACTORY.valueOf(uuid);
}
- public static RaftGroupId valueOf(ByteString data) {
- return new RaftGroupId(data);
+ public static RaftGroupId valueOf(ByteString bytes) {
+ return FACTORY.valueOf(bytes);
}
private RaftGroupId(UUID id) {
super(id);
}
- private RaftGroupId(ByteString data) {
- super(data);
+ private RaftGroupId(UUID uuid, ByteString bytes) {
+ super(uuid, bytes);
}
@Override
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
index b0f6228b2..f1d07a980 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftId.java
@@ -17,13 +17,17 @@
*/
package org.apache.ratis.protocol;
+import org.apache.ratis.thirdparty.com.google.common.cache.Cache;
+import org.apache.ratis.thirdparty.com.google.common.cache.CacheBuilder;
import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteStringUtils;
import org.apache.ratis.util.JavaUtils;
import org.apache.ratis.util.Preconditions;
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.UUID;
+import java.util.concurrent.ExecutionException;
import java.util.function.Supplier;
/** Unique identifier implemented using {@link UUID}. */
@@ -32,24 +36,52 @@ public abstract class RaftId {
static final ByteString ZERO_UUID_BYTESTRING = toByteString(ZERO_UUID);
private static final int BYTE_LENGTH = 16;
- private static void checkLength(int length, String name) {
- Preconditions.assertTrue(length == BYTE_LENGTH,
- "%s = %s != BYTE_LENGTH = %s", name, length, BYTE_LENGTH);
- }
-
- private static UUID toUuid(ByteString bytes) {
+ static UUID toUuid(ByteString bytes) {
Objects.requireNonNull(bytes, "bytes == null");
- checkLength(bytes.size(), "bytes.size()");
+ Preconditions.assertSame(BYTE_LENGTH, bytes.size(), "bytes.size()");
final ByteBuffer buf = bytes.asReadOnlyByteBuffer();
return new UUID(buf.getLong(), buf.getLong());
}
- private static ByteString toByteString(UUID uuid) {
+ static ByteString toByteString(UUID uuid) {
Objects.requireNonNull(uuid, "uuid == null");
- final ByteBuffer buf = ByteBuffer.wrap(new byte[BYTE_LENGTH]);
- buf.putLong(uuid.getMostSignificantBits());
- buf.putLong(uuid.getLeastSignificantBits());
- return ByteString.copyFrom(buf.array());
+ final byte[] array = new byte[BYTE_LENGTH];
+ ByteBuffer.wrap(array)
+ .putLong(uuid.getMostSignificantBits())
+ .putLong(uuid.getLeastSignificantBits());
+ return ByteStringUtils.unsafeWrap(array);
+ }
+
+ abstract static class Factory<ID extends RaftId> {
+ private final Cache<UUID, ID> cache = CacheBuilder.newBuilder()
+ .weakValues()
+ .build();
+
+ abstract ID newInstance(UUID uuid, ByteString bytes);
+
+ private ID valueOf(UUID uuid, ByteString bytes) {
+ try {
+ return cache.get(uuid, () -> newInstance(uuid, bytes));
+ } catch (ExecutionException e) {
+ throw new IllegalStateException("Failed to valueOf(" + uuid + ")", e);
+ }
+ }
+
+ final ID valueOf(UUID uuid) {
+ return valueOf(uuid, null);
+ }
+
+ final ID valueOf(ByteString bytes) {
+ return bytes != null? valueOf(toUuid(bytes), bytes): emptyId();
+ }
+
+ ID emptyId() {
+ return valueOf(ZERO_UUID, ZERO_UUID_BYTESTRING);
+ }
+
+ ID randomId() {
+ return valueOf(UUID.randomUUID(), null);
+ }
}
private final UUID uuid;
@@ -68,8 +100,9 @@ public abstract class RaftId {
() -> "Failed to create " + JavaUtils.getClassSimpleName(getClass()) +
": UUID " + ZERO_UUID + " is reserved.");
}
- RaftId(ByteString uuidBytes) {
- this(toUuid(uuidBytes), () -> uuidBytes);
+ RaftId(UUID uuid, ByteString bytes) {
+ this(uuid, () -> bytes);
+ Preconditions.assertTrue(toUuid(bytes).equals(uuid));
}
/** @return the last 12 hex digits. */
diff --git
a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeerId.java
b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeerId.java
index 5da89691b..afb7ef357 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeerId.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftPeerId.java
@@ -29,8 +29,8 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;
/**
- * Id of Raft Peer which is globally unique.
- *
+ * ID of Raft Peer which is globally unique.
+ * <p>
* This is a value-based class.
*/
public final class RaftPeerId {
@@ -64,14 +64,13 @@ public final class RaftPeerId {
private RaftPeerId(ByteString id) {
this.id = Objects.requireNonNull(id, "id == null");
- Preconditions.assertTrue(id.size() > 0, "id is empty.");
+ Preconditions.assertTrue(!id.isEmpty(), "id is empty.");
this.idString = id.toString(StandardCharsets.UTF_8);
this.raftPeerIdProto = JavaUtils.memoize(this::buildRaftPeerIdProto);
}
private RaftPeerIdProto buildRaftPeerIdProto() {
- final RaftPeerIdProto.Builder builder =
RaftPeerIdProto.newBuilder().setId(id);
- return builder.build();
+ return RaftPeerIdProto.newBuilder().setId(id).build();
}
public RaftPeerIdProto getRaftPeerIdProto() {
diff --git
a/ratis-common/src/main/java/org/apache/ratis/thirdparty/com/google/protobuf/ByteStringUtils.java
b/ratis-common/src/main/java/org/apache/ratis/thirdparty/com/google/protobuf/ByteStringUtils.java
new file mode 100644
index 000000000..85174979f
--- /dev/null
+++
b/ratis-common/src/main/java/org/apache/ratis/thirdparty/com/google/protobuf/ByteStringUtils.java
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.thirdparty.com.google.protobuf;
+
+/** Utilities for {@link ByteString}. */
+public interface ByteStringUtils {
+ /**
+ * Wrap the given array.
+ * Note that the content of the array must not be changed.
+ * Otherwise, it violates the immutability of {@link ByteString}.
+ */
+ static ByteString unsafeWrap(byte[] array) {
+ return ByteString.wrap(array);
+ }
+}
diff --git a/ratis-test/src/test/java/org/apache/ratis/protocol/TestRaftId.java
b/ratis-test/src/test/java/org/apache/ratis/protocol/TestRaftId.java
index 30b7ed54f..6610b3d04 100644
--- a/ratis-test/src/test/java/org/apache/ratis/protocol/TestRaftId.java
+++ b/ratis-test/src/test/java/org/apache/ratis/protocol/TestRaftId.java
@@ -1,4 +1,4 @@
-/**
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -22,12 +22,31 @@ import
org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
import org.junit.Assert;
import org.junit.Test;
+import java.util.UUID;
+
public class TestRaftId extends BaseTest {
@Override
public int getGlobalTimeoutSeconds() {
return 1;
}
+ @Test
+ public void testRaftId() {
+ assertRaftId(RaftId.ZERO_UUID, RaftId.ZERO_UUID_BYTESTRING);
+ assertRaftId(UUID.randomUUID(), null);
+ }
+
+ static void assertRaftId(UUID original, ByteString expected) {
+ final ByteString bytes = RaftId.toByteString(original);
+ if (expected != null) {
+ Assert.assertEquals(expected, bytes);
+ }
+ final UUID computed = RaftId.toUuid(bytes);
+
+ Assert.assertEquals(original, computed);
+ Assert.assertEquals(bytes, RaftId.toByteString(computed));
+ }
+
@Test
public void testClientId() {
final ClientId id = ClientId.randomId();
