adutra commented on code in PR #2131: URL: https://github.com/apache/polaris/pull/2131#discussion_r2215586374
########## persistence/nosql/idgen/api/src/main/java/org/apache/polaris/ids/api/IdGeneratorSpec.java: ########## @@ -0,0 +1,52 @@ +/* + * 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.polaris.ids.api; + +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; +import io.smallrye.config.WithDefault; +import java.util.Map; +import org.apache.polaris.immutables.PolarisImmutable; +import org.immutables.value.Value; + +@PolarisImmutable +@JsonSerialize(as = ImmutableIdGeneratorSpec.class) +@JsonDeserialize(as = ImmutableIdGeneratorSpec.class) +public interface IdGeneratorSpec { + @WithDefault("snowflake") + String type(); + + Map<String, String> params(); + + @PolarisImmutable + interface BuildableIdGeneratorSpec extends IdGeneratorSpec { Review Comment: This class looks odd? Afaict we could use `ImmutableIdGeneratorSpec.builder()` instead. The only difference seems to be that there is a default type here, whereas in `IdGeneratorSpec` there isn't, but it's certainly possible to overcome this limitation somehow. ########## persistence/nosql/idgen/api/src/main/java/org/apache/polaris/ids/api/MonotonicClock.java: ########## @@ -0,0 +1,69 @@ +/* + * 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.polaris.ids.api; + +import java.time.Instant; + +/** + * Provides a clock providing the current time in milliseconds, microseconds and instant since + * 1970-01-01-00:00:00.000. The returned timestamp values increase monotonically. + * + * <p>The functions provide nanosecond/microsecond/millisecond precision, but not necessarily the + * same resolution (how frequently the value changes) - no guarantees are made. + * + * <p>Implementation <em>may</em> adjust to wall clocks advancing faster than the real time. If and + * how exactly depends on the implementation, as long as none of the time values available via this + * interface "goes backwards". + * + * <p>Implementer notes: {@link System#nanoTime() System.nanoTime()} does not guarantee that the + * values will be monotonically increasing when invocations happen from different + * CPUs/cores/threads. + * + * <p>A default implementation of {@link MonotonicClock} can be injected as an application scoped + * bean in CDI. + */ +public interface MonotonicClock extends AutoCloseable { + /** + * Current timestamp as microseconds since epoch, can be used as a monotonically increasing wall + * clock. + */ + long currentTimeMicros(); + + /** + * Current timestamp as milliseconds since epoch, can be used as a monotonically increasing wall + * clock. + */ + long currentTimeMillis(); + + /** + * Current instant with nanosecond precision, can be used as a monotonically increasing wall + * clock. + */ + Instant currentInstant(); + + /** Monotonically increasing timestamp with nanosecond precision, not related to wall clock. */ + long nanoTime(); + + void sleepMillis(long millis); + + @Override + void close(); + + void waitUntilTimeMillisAdvanced(); Review Comment: Could be good to add javadocs here (and above), it's not immediately clear what this method is supposed to do (spin-wait until the clock ticks?). Also neither this method nor `sleepMillis` throw `InterruptedException`, which is surprising since they are clearly blocking. It could be good to add an `@implSpec` note about how the interrupt flag is expected to be handled. ########## persistence/nosql/idgen/spi/build.gradle.kts: ########## @@ -0,0 +1,44 @@ +/* + * 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. + */ + +plugins { + alias(libs.plugins.jandex) + id("polaris-server") +} + +description = "Polaris ID generation SPI" + +dependencies { + implementation(project(":polaris-idgen-api")) + + compileOnly(libs.jakarta.annotation.api) + compileOnly(libs.jakarta.validation.api) + compileOnly(libs.jakarta.inject.api) + compileOnly(libs.jakarta.enterprise.cdi.api) + + compileOnly(libs.smallrye.config.core) + compileOnly(platform(libs.quarkus.bom)) + compileOnly("io.quarkus:quarkus-core") + + compileOnly(project(":polaris-immutables")) Review Comment: This dependency seems unused. I wonder if you didn't forget to annotate `IdGeneratorSource` with `@PolarisImmutable`? It seems like a good candidate for that (lots of anonymous classes implementing this interface). ########## persistence/nosql/idgen/impl/src/main/java/org/apache/polaris/ids/impl/SnowflakeIdGeneratorImpl.java: ########## @@ -0,0 +1,408 @@ +/* + * 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.polaris.ids.impl; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; + +import com.google.common.annotations.VisibleForTesting; +import jakarta.annotation.Nonnull; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicLongFieldUpdater; +import org.apache.polaris.ids.api.MonotonicClock; +import org.apache.polaris.ids.api.SnowflakeIdGenerator; +import org.apache.polaris.ids.spi.IdGeneratorSource; + +/** + * Implementation of a local, per-node generator for so-called "snowflake IDs", which are unique + * integer IDs in a distributed environment. + * + * <p>A monotonically increasing clock is <em>strictly required</em>. Invocations of {@link + * #generateId()} fail hard, if the clock walks backwards, which means it returns a lower value than + * before. It is recommended to use an implementation of {@link MonotonicClock} as the clock source. + * + * <p>The implementation is thread-safe. + * + * <p>Reference: <a + * href="https://medium.com/@jitenderkmr/demystifying-snowflake-ids-a-unique-identifier-in-distributed-computing-72796a827c9d">Article + * on medium.com</a>, <a + * href="https://github.com/twitter-archive/snowflake/tree/b3f6a3c6ca8e1b6847baa6ff42bf72201e2c2231">Twitter + * GitHub repository (archived)</a> + */ +class SnowflakeIdGeneratorImpl implements SnowflakeIdGenerator { + + // TODO add a specialized implementation using hard-coded values for the standardized parameters + + private static final AtomicLongFieldUpdater<SnowflakeIdGeneratorImpl> LAST_ID_UPDATER = + AtomicLongFieldUpdater.newUpdater(SnowflakeIdGeneratorImpl.class, "lastId"); + + private final IdGeneratorSource idGeneratorSource; + + // Used in hot generateId() + private volatile long lastId; + private final long epochOffset; + private final long timestampMax; + private final int timestampShift; + private final int sequenceBits; + private final long sequenceMask; + private final long nodeMask; + + SnowflakeIdGeneratorImpl(IdGeneratorSource idGeneratorSource) { + this( + DEFAULT_TIMESTAMP_BITS, + DEFAULT_SEQUENCE_BITS, + DEFAULT_NODE_ID_BITS, + EPOCH_OFFSET_MILLIS, + idGeneratorSource); + } + + SnowflakeIdGeneratorImpl( + int timestampBits, + int sequenceBits, + int nodeBits, + long epochOffset, + IdGeneratorSource idGeneratorSource) { + validateArguments(timestampBits, sequenceBits, nodeBits, epochOffset, idGeneratorSource); + this.timestampShift = sequenceBits + nodeBits; + this.timestampMax = 1L << timestampBits; + this.nodeMask = (1L << nodeBits) - 1; + this.sequenceBits = sequenceBits; + this.sequenceMask = (1L << sequenceBits) - 1; + this.epochOffset = epochOffset; + this.idGeneratorSource = idGeneratorSource; + } + + static void validateArguments( + int timestampBits, + int sequenceBits, + int nodeBits, + long epochOffset, + IdGeneratorSource idGeneratorSource) { + var nowMillis = idGeneratorSource != null ? idGeneratorSource.currentTimeMillis() : -1; + var now = Instant.ofEpochMilli(nowMillis); + var timestampMax = 1L << timestampBits; + checkArgs( + () -> checkArgument(idGeneratorSource != null, "IdGeneratorSource must not be null"), + () -> + checkArgument( + nowMillis >= epochOffset, + "Clock returns a timestamp %s less than the configured epoch %s", + now, + Instant.ofEpochMilli(epochOffset)), + () -> + checkArgument( + nowMillis - epochOffset < timestampMax, + "Clock already returns a timestamp %s greater of after %s", + now, + Instant.ofEpochMilli(timestampMax)), + () -> + checkArgument( + nodeBits >= 2 + && sequenceBits >= 5 + && timestampBits >= 5 // this is REALLY low ! + && nodeBits < 64 + && sequenceBits < 64 + && timestampBits < 64, + "value of nodeBits %s or sequenceBits %s or timestampBits %s is too low or too high", + nodeBits, + sequenceBits, + timestampBits), + () -> + checkArgument( + timestampBits + nodeBits + sequenceBits == 63, + "Sum of timestampBits + nodeBits + sequenceBits must be == 63"), + () -> { + if (idGeneratorSource != null) { + var nodeId = idGeneratorSource.nodeId(); + var nodeMax = 1L << nodeBits; + checkArgument( + nodeId >= 0 && nodeId < nodeMax, "nodeId %s out of range [0..%s[", nodeId, nodeMax); + } + }); + } + + static void checkArgs(Runnable... checks) { + var violations = new ArrayList<String>(); + for (Runnable check : checks) { + try { + check.run(); + } catch (IllegalArgumentException iae) { + violations.add(iae.getMessage()); + } + } + if (!violations.isEmpty()) { + throw new IllegalArgumentException(String.join(", ", violations)); + } + } + + @Override + public long systemIdForNode(int nodeId) { + return constructIdUnsafe(timestampMax - 1, 0, nodeId); + } + + private long constructIdUnsafe(long timestamp, long sequence, long nodeId) { + return (timestamp << timestampShift) | (nodeId << sequenceBits) | sequence; + } + + @Override + public long constructId(long timestamp, long sequence, long nodeId) { Review Comment: This method effectively bypasses the Id generator source. Maybe it shouldn't be public? Having two methods in the same API that generate IDs using different mechanisms seems error-prone. OTOH this method seems only called from `timeUuidToId`, in this same class, so it seems it could be private. ########## persistence/nosql/idgen/impl/src/main/java/org/apache/polaris/ids/impl/SnowflakeIdGeneratorFactory.java: ########## @@ -0,0 +1,107 @@ +/* + * 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.polaris.ids.impl; + +import static org.apache.polaris.ids.impl.SnowflakeIdGeneratorImpl.validateArguments; + +import java.time.Instant; +import java.util.Map; +import java.util.function.LongSupplier; +import org.apache.polaris.ids.api.SnowflakeIdGenerator; +import org.apache.polaris.ids.spi.IdGeneratorFactory; +import org.apache.polaris.ids.spi.IdGeneratorSource; + +public class SnowflakeIdGeneratorFactory implements IdGeneratorFactory<SnowflakeIdGenerator> { + @Override + public void validateParameters(Map<String, String> params, IdGeneratorSource idGeneratorSource) { + int timestampBits = + Integer.parseInt( + params.getOrDefault( + "timestamp-bits", "" + SnowflakeIdGenerator.DEFAULT_TIMESTAMP_BITS)); + int nodeIdBits = + Integer.parseInt( + params.getOrDefault("node-id-bits", "" + SnowflakeIdGenerator.DEFAULT_NODE_ID_BITS)); + int sequenceBits = + Integer.parseInt( + params.getOrDefault("sequence-bits", "" + SnowflakeIdGenerator.DEFAULT_SEQUENCE_BITS)); + var offsetMillis = SnowflakeIdGenerator.EPOCH_OFFSET_MILLIS; + var offset = params.get("offset"); + if (offset != null) { + offsetMillis = Instant.parse(offset).toEpochMilli(); + } + + validateArguments(timestampBits, sequenceBits, nodeIdBits, offsetMillis, idGeneratorSource); + } + + @Override + public SnowflakeIdGenerator buildSystemIdGenerator( + Map<String, String> params, LongSupplier clockMillis) { + return buildIdGenerator( + params, + new IdGeneratorSource() { + @Override + public int nodeId() { + return 0; + } + + @Override + public long currentTimeMillis() { + return SnowflakeIdGenerator.EPOCH_OFFSET_MILLIS; Review Comment: This looks suspicious as it is returning a fixed timestamp. Did you mean `clockMillis.getAsLong()`? ```suggestion return clockMillis.getAsLong(); ``` ########## persistence/nosql/idgen/api/src/main/java/org/apache/polaris/ids/api/SnowflakeIdGenerator.java: ########## @@ -0,0 +1,62 @@ +/* + * 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.polaris.ids.api; + +import jakarta.annotation.Nonnull; +import java.time.Instant; +import java.time.ZoneId; +import java.util.UUID; + +public interface SnowflakeIdGenerator extends IdGenerator { + /** Offset of the snowflake ID generator since the 1970-01-01T00:00:00Z epoch instant. */ + Instant EPOCH_OFFSET = + Instant.EPOCH.atZone(ZoneId.of("GMT")).withYear(2025).withMonth(3).toInstant(); + + /** + * Offset of the snowflake ID generator in milliseconds since the 1970-01-01T00:00:00Z epoch + * instant. + */ + long EPOCH_OFFSET_MILLIS = EPOCH_OFFSET.toEpochMilli(); + + int DEFAULT_NODE_ID_BITS = 10; + int DEFAULT_TIMESTAMP_BITS = 41; + int DEFAULT_SEQUENCE_BITS = 12; + + long constructId(long timestamp, long sequence, long node); + + long timestampFromId(long id); Review Comment: I suppose this is understood as "since the Snowflake Epoch (2025-03-01)". Could be good to add javadocs to clarify. ########## persistence/nosql/idgen/api/src/main/java/org/apache/polaris/ids/api/SnowflakeIdGenerator.java: ########## @@ -0,0 +1,62 @@ +/* + * 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.polaris.ids.api; + +import jakarta.annotation.Nonnull; +import java.time.Instant; +import java.time.ZoneId; +import java.util.UUID; + +public interface SnowflakeIdGenerator extends IdGenerator { + /** Offset of the snowflake ID generator since the 1970-01-01T00:00:00Z epoch instant. */ + Instant EPOCH_OFFSET = + Instant.EPOCH.atZone(ZoneId.of("GMT")).withYear(2025).withMonth(3).toInstant(); + + /** + * Offset of the snowflake ID generator in milliseconds since the 1970-01-01T00:00:00Z epoch + * instant. + */ + long EPOCH_OFFSET_MILLIS = EPOCH_OFFSET.toEpochMilli(); + + int DEFAULT_NODE_ID_BITS = 10; + int DEFAULT_TIMESTAMP_BITS = 41; + int DEFAULT_SEQUENCE_BITS = 12; + + long constructId(long timestamp, long sequence, long node); + + long timestampFromId(long id); + + long timestampUtcFromId(long id); Review Comment: "Timestamp UTC" does not make sense to me 🤔 From the implementation class, it seems you meant "timestamp since Unix Epoch" instead. ########## persistence/nosql/idgen/api/src/main/java/org/apache/polaris/ids/api/SnowflakeIdGenerator.java: ########## @@ -0,0 +1,62 @@ +/* + * 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.polaris.ids.api; + +import jakarta.annotation.Nonnull; +import java.time.Instant; +import java.time.ZoneId; +import java.util.UUID; + +public interface SnowflakeIdGenerator extends IdGenerator { + /** Offset of the snowflake ID generator since the 1970-01-01T00:00:00Z epoch instant. */ + Instant EPOCH_OFFSET = Review Comment: Also, the expression is unnecessarily complex, the below one is 100% equivalent and imho easier to grasp: ```java Instant.parse("2025-03-01T00:00:00Z") ``` ########## persistence/nosql/idgen/impl/src/main/java/org/apache/polaris/ids/impl/SnowflakeIdGeneratorImpl.java: ########## @@ -0,0 +1,408 @@ +/* + * 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.polaris.ids.impl; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static java.lang.String.format; + +import com.google.common.annotations.VisibleForTesting; +import jakarta.annotation.Nonnull; +import java.time.Instant; +import java.time.LocalDateTime; +import java.time.ZoneOffset; +import java.util.ArrayList; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicLongFieldUpdater; +import org.apache.polaris.ids.api.MonotonicClock; +import org.apache.polaris.ids.api.SnowflakeIdGenerator; +import org.apache.polaris.ids.spi.IdGeneratorSource; + +/** + * Implementation of a local, per-node generator for so-called "snowflake IDs", which are unique + * integer IDs in a distributed environment. + * + * <p>A monotonically increasing clock is <em>strictly required</em>. Invocations of {@link + * #generateId()} fail hard, if the clock walks backwards, which means it returns a lower value than + * before. It is recommended to use an implementation of {@link MonotonicClock} as the clock source. + * + * <p>The implementation is thread-safe. + * + * <p>Reference: <a + * href="https://medium.com/@jitenderkmr/demystifying-snowflake-ids-a-unique-identifier-in-distributed-computing-72796a827c9d">Article + * on medium.com</a>, <a + * href="https://github.com/twitter-archive/snowflake/tree/b3f6a3c6ca8e1b6847baa6ff42bf72201e2c2231">Twitter + * GitHub repository (archived)</a> + */ +class SnowflakeIdGeneratorImpl implements SnowflakeIdGenerator { + + // TODO add a specialized implementation using hard-coded values for the standardized parameters + + private static final AtomicLongFieldUpdater<SnowflakeIdGeneratorImpl> LAST_ID_UPDATER = + AtomicLongFieldUpdater.newUpdater(SnowflakeIdGeneratorImpl.class, "lastId"); + + private final IdGeneratorSource idGeneratorSource; + + // Used in hot generateId() + private volatile long lastId; + private final long epochOffset; + private final long timestampMax; + private final int timestampShift; + private final int sequenceBits; + private final long sequenceMask; + private final long nodeMask; + + SnowflakeIdGeneratorImpl(IdGeneratorSource idGeneratorSource) { + this( + DEFAULT_TIMESTAMP_BITS, + DEFAULT_SEQUENCE_BITS, + DEFAULT_NODE_ID_BITS, + EPOCH_OFFSET_MILLIS, + idGeneratorSource); + } + + SnowflakeIdGeneratorImpl( + int timestampBits, + int sequenceBits, + int nodeBits, + long epochOffset, + IdGeneratorSource idGeneratorSource) { + validateArguments(timestampBits, sequenceBits, nodeBits, epochOffset, idGeneratorSource); + this.timestampShift = sequenceBits + nodeBits; + this.timestampMax = 1L << timestampBits; + this.nodeMask = (1L << nodeBits) - 1; + this.sequenceBits = sequenceBits; + this.sequenceMask = (1L << sequenceBits) - 1; + this.epochOffset = epochOffset; + this.idGeneratorSource = idGeneratorSource; + } + + static void validateArguments( + int timestampBits, + int sequenceBits, + int nodeBits, + long epochOffset, + IdGeneratorSource idGeneratorSource) { + var nowMillis = idGeneratorSource != null ? idGeneratorSource.currentTimeMillis() : -1; + var now = Instant.ofEpochMilli(nowMillis); + var timestampMax = 1L << timestampBits; + checkArgs( + () -> checkArgument(idGeneratorSource != null, "IdGeneratorSource must not be null"), + () -> + checkArgument( + nowMillis >= epochOffset, + "Clock returns a timestamp %s less than the configured epoch %s", + now, + Instant.ofEpochMilli(epochOffset)), + () -> + checkArgument( + nowMillis - epochOffset < timestampMax, + "Clock already returns a timestamp %s greater of after %s", + now, + Instant.ofEpochMilli(timestampMax)), + () -> + checkArgument( + nodeBits >= 2 + && sequenceBits >= 5 + && timestampBits >= 5 // this is REALLY low ! + && nodeBits < 64 + && sequenceBits < 64 + && timestampBits < 64, + "value of nodeBits %s or sequenceBits %s or timestampBits %s is too low or too high", + nodeBits, + sequenceBits, + timestampBits), + () -> + checkArgument( + timestampBits + nodeBits + sequenceBits == 63, + "Sum of timestampBits + nodeBits + sequenceBits must be == 63"), + () -> { + if (idGeneratorSource != null) { + var nodeId = idGeneratorSource.nodeId(); + var nodeMax = 1L << nodeBits; + checkArgument( + nodeId >= 0 && nodeId < nodeMax, "nodeId %s out of range [0..%s[", nodeId, nodeMax); + } + }); + } + + static void checkArgs(Runnable... checks) { + var violations = new ArrayList<String>(); + for (Runnable check : checks) { + try { + check.run(); + } catch (IllegalArgumentException iae) { + violations.add(iae.getMessage()); + } + } + if (!violations.isEmpty()) { + throw new IllegalArgumentException(String.join(", ", violations)); + } + } + + @Override + public long systemIdForNode(int nodeId) { + return constructIdUnsafe(timestampMax - 1, 0, nodeId); + } + + private long constructIdUnsafe(long timestamp, long sequence, long nodeId) { + return (timestamp << timestampShift) | (nodeId << sequenceBits) | sequence; + } + + @Override + public long constructId(long timestamp, long sequence, long nodeId) { + checkArgument( + (timestamp & (timestampMax - 1)) != timestampMax - 1, + "timestamp argument %s out of range", + timestamp); + checkArgument( + (sequence & sequenceMask) == sequence, "sequence argument %s out of range", sequence); + checkArgument((nodeId & nodeMask) == nodeId, "nodeId argument %s out of range", nodeId); + return constructIdUnsafe(timestamp, sequence, nodeId); + } + + @Override + public long generateId() { + var nodeId = idGeneratorSource.nodeId(); + checkState(nodeId >= 0, "Cannot generate a new ID, shutting down?"); + var nodeIdPattern = ((long) nodeId) << sequenceBits; Review Comment: Nit: move this variable declaration to line 228. It's not needed before. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
