Skip to content

Commit 588a26d

Browse files
committed
change generator logic, per @adutra
1 parent f44b890 commit 588a26d

File tree

2 files changed

+36
-27
lines changed

2 files changed

+36
-27
lines changed

runtime/service/src/main/java/org/apache/polaris/service/tracing/RequestIdGenerator.java

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,32 +22,35 @@
2222
import com.google.common.annotations.VisibleForTesting;
2323
import jakarta.enterprise.context.ApplicationScoped;
2424
import java.util.UUID;
25-
import java.util.concurrent.atomic.AtomicLong;
25+
import java.util.concurrent.atomic.AtomicReference;
2626

2727
@ApplicationScoped
2828
public class RequestIdGenerator {
29-
private volatile String baseDefaultUuid = UUID.randomUUID().toString();
30-
static final AtomicLong COUNTER = new AtomicLong();
3129
static final Long COUNTER_SOFT_MAX = Long.MAX_VALUE / 2;
3230

33-
public String generateRequestId() {
34-
long currentCounter = COUNTER.incrementAndGet();
35-
String requestId = baseDefaultUuid + "_" + currentCounter;
36-
if (currentCounter >= COUNTER_SOFT_MAX) {
37-
synchronized (this) {
38-
if (COUNTER.get() >= COUNTER_SOFT_MAX) {
39-
// If we get anywhere close to danger of overflowing Long (which, theoretically,
40-
// would be extremely unlikely) rotate the UUID and start again.
41-
baseDefaultUuid = UUID.randomUUID().toString();
42-
COUNTER.set(0);
43-
}
44-
}
31+
record State(String uuid, long counter) {
32+
33+
State() {
34+
this(UUID.randomUUID().toString(), 1);
35+
}
36+
37+
String requestId() {
38+
return String.format("%s_%019d", uuid, counter);
39+
}
40+
41+
State increment() {
42+
return counter >= COUNTER_SOFT_MAX ? new State() : new State(uuid, counter + 1);
4543
}
46-
return requestId;
44+
}
45+
46+
final AtomicReference<State> state = new AtomicReference<>(new State());
47+
48+
public String generateRequestId() {
49+
return state.getAndUpdate(State::increment).requestId();
4750
}
4851

4952
@VisibleForTesting
5053
public void setCounter(long counter) {
51-
COUNTER.set(counter);
54+
state.set(new State(state.get().uuid, counter));
5255
}
5356
}

runtime/service/src/test/java/org/apache/polaris/service/tracing/RequestIdGeneratorTest.java

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ void testGenerateRequestId_ReturnsValidFormat() {
4343

4444
assertThat(requestId).isNotNull();
4545
assertThat(requestId).matches(this::isValidRequestIdFormat);
46-
assertThat(requestId).contains("_1"); // First call should increment counter to 1
46+
// First call should increment counter to 1
47+
assertThat(extractCounterFromRequestId(requestId)).isEqualTo(1);
4748
}
4849

4950
@Test
@@ -62,22 +63,22 @@ void testGenerateRequestId_ReturnsUniqueIds() {
6263

6364
@Test
6465
void testCounterIncrementsSequentially() {
65-
requestIdGenerator.setCounter(0);
66+
// requestIdGenerator.setCounter(0);
6667

6768
String firstId = requestIdGenerator.generateRequestId();
6869
String secondId = requestIdGenerator.generateRequestId();
6970
String thirdId = requestIdGenerator.generateRequestId();
7071

71-
assertThat(firstId).endsWith("_1");
72-
assertThat(secondId).endsWith("_2");
73-
assertThat(thirdId).endsWith("_3");
72+
assertThat(extractCounterFromRequestId(firstId)).isEqualTo(1);
73+
assertThat(extractCounterFromRequestId(secondId)).isEqualTo(2);
74+
assertThat(extractCounterFromRequestId(thirdId)).isEqualTo(3);
7475
}
7576

7677
@Test
7778
void testCounterRotationAtSoftMax() {
7879
// Set counter close to soft max
7980
long softMax = RequestIdGenerator.COUNTER_SOFT_MAX;
80-
requestIdGenerator.setCounter(softMax - 1);
81+
requestIdGenerator.setCounter(softMax);
8182

8283
String beforeRotation = requestIdGenerator.generateRequestId();
8384
String afterRotation = requestIdGenerator.generateRequestId();
@@ -87,9 +88,9 @@ void testCounterRotationAtSoftMax() {
8788
String afterUuidPart = afterRotation.substring(0, afterRotation.lastIndexOf('_'));
8889
assertNotEquals(beforeUuidPart, afterUuidPart);
8990

90-
// After rotation, counter should be reset and UUID should be different
91-
assertThat(beforeRotation).endsWith("_" + softMax);
92-
assertThat(afterRotation).endsWith("_1"); // Counter reset to 1 (after increment from 0)
91+
assertThat(extractCounterFromRequestId(beforeRotation)).isEqualTo(softMax);
92+
// Counter reset to 1 (after increment from 0)
93+
assertThat(extractCounterFromRequestId(afterRotation)).isEqualTo(1);
9394
}
9495

9596
@Test
@@ -98,7 +99,8 @@ void testSetCounterChangesNextGeneratedId() {
9899

99100
String requestId = requestIdGenerator.generateRequestId();
100101

101-
assertThat(requestId).endsWith("_101"); // Should increment from set value
102+
// Should increment from set value
103+
assertThat(extractCounterFromRequestId(requestId)).isEqualTo(100);
102104
}
103105

104106
private boolean isValidRequestIdFormat(String str) {
@@ -113,4 +115,8 @@ private boolean isValidRequestIdFormat(String str) {
113115
return false;
114116
}
115117
}
118+
119+
private long extractCounterFromRequestId(String requestId) {
120+
return Long.parseLong(requestId.split("_")[1]);
121+
}
116122
}

0 commit comments

Comments
 (0)