blerer commented on a change in pull request #1276:
URL: https://github.com/apache/cassandra/pull/1276#discussion_r756904423



##########
File path: CHANGES.txt
##########
@@ -1,4 +1,5 @@
 4.1
+ * UUID based sstable generation identifiers (CASSANDRA-17048)

Review comment:
       Should probably be `Add support for UUID based sstable generation 
identifiers` we do not enforce the change people will be still free to use the 
previous approach.

##########
File path: src/java/org/apache/cassandra/io/sstable/SSTableUniqueIdentifier.java
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+/**
+ * Represents a unique identifier in the sstable descriptor filename.
+ * This ensures each sstable files uniqueness in the system.
+ * <p>
+ * A new iteration implementation must adhere to the following invariants:
+ * - Must be roughly sortable (for determinism)

Review comment:
       `roughly` is confusing. The explication should be more precise and 
should probably contains a justification of why it is required. 

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SSTableUniqueIdentifierFactory.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+
+import org.apache.cassandra.config.DatabaseDescriptor;
+
+public class SSTableUniqueIdentifierFactory
+{
+    public static final SSTableUniqueIdentifierFactory instance = new 
SSTableUniqueIdentifierFactory();
+
+    /**
+     * Tries to guess the exact identifier type and create an instance of 
{@link SSTableUniqueIdentifier} using
+     * {@link SSTableUniqueIdentifier.Builder#fromString(String)} from the 
guessed builder
+     *
+     * @throws IllegalArgumentException when the provided string 
representation is malformed
+     */
+    public SSTableUniqueIdentifier fromString(String str) throws 
IllegalArgumentException
+    {
+        SSTableUniqueIdentifier.Builder<?> builder = str.length() == 
UUIDBasedSSTableUniqueIdentifier.STRING_LEN
+                                                     ? 
UUIDBasedSSTableUniqueIdentifier.Builder.instance
+                                                     : 
SequenceBasedSSTableUniqueIdentifier.Builder.instance;
+        return builder.fromString(str);
+    }
+
+    /**
+     * Tries to guess the exact identifier type and create an instance of 
{@link SSTableUniqueIdentifier} using
+     * {@link SSTableUniqueIdentifier.Builder#fromBytes(ByteBuffer)} from the 
guessed builder
+     *
+     * @throws IllegalArgumentException whenn the provided binary 
representation is malformed
+     */
+    public SSTableUniqueIdentifier fromBytes(ByteBuffer bytes)
+    {
+        SSTableUniqueIdentifier.Builder<?> builder = bytes.remaining() == 
UUIDBasedSSTableUniqueIdentifier.BYTES_LEN

Review comment:
       The code seems to expect the buffer to contains only identifier bytes 
but it is not clearly mentioned. If it is not the case we can end upd with the 
wrong identifier being returned in a silent way. Same thing if for some reason 
the buffer contains an unexpected number of bytes (10 for example). 

##########
File path: 
test/unit/org/apache/cassandra/io/sstable/SSTableUniqueIdentifierTest.java
##########
@@ -0,0 +1,92 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.UUID;
+import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+import com.google.common.collect.Sets;
+import com.google.common.primitives.UnsignedBytes;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.quicktheories.QuickTheory.qt;
+import static org.quicktheories.generators.SourceDSL.longs;
+
+public class SSTableUniqueIdentifierTest
+{
+    @Test
+    public void testSequenceBasedIdProperties()
+    {
+        
testSSTableUniqueIdentifierProperties(SequenceBasedSSTableUniqueIdentifier.Builder.instance);
+    }
+
+    @Test
+    public void testUUIDBasedIdProperties()
+    {
+        
testSSTableUniqueIdentifierProperties(UUIDBasedSSTableUniqueIdentifier.Builder.instance);
+    }
+
+    private void 
testSSTableUniqueIdentifierProperties(SSTableUniqueIdentifier.Builder<?> 
builder)
+    {
+        List<SSTableUniqueIdentifier> ids = 
Stream.generate(builder.generator(Stream.empty()))
+                                                  
.limit(100).collect(Collectors.toList());
+        assertThat(ids).isSorted();
+        assertThat(Sets.newHashSet(ids)).hasSameSizeAs(ids);
+
+        List<ByteBuffer> serIds = 
ids.stream().map(SSTableUniqueIdentifier::asBytes).collect(Collectors.toList());
+        assertThat(serIds).isSortedAccordingTo((o1, o2) -> 
UnsignedBytes.lexicographicalComparator().compare(o1.array(), o2.array()));
+
+        List<SSTableUniqueIdentifier> deserIds = 
serIds.stream().map(builder::fromBytes).collect(Collectors.toList());
+        assertThat(deserIds).containsExactlyElementsOf(ids);
+
+        List<String> stringifiedIds = 
ids.stream().map(SSTableUniqueIdentifier::asString).collect(Collectors.toList());
+        if (!(builder instanceof SequenceBasedSSTableUniqueIdentifier.Builder))
+        {
+            // the legacy string representation is not sortable
+            assertThat(stringifiedIds).isSorted();
+        }
+
+        List<SSTableUniqueIdentifier> destringifiedIds = 
stringifiedIds.stream().map(builder::fromString).collect(Collectors.toList());
+        assertThat(destringifiedIds).containsExactlyElementsOf(ids);
+    }
+
+    @Test
+    public void testUUID()
+    {
+        qt().forAll(longs().all(), longs().all()).checkAssert((msb, lsb) -> {
+            msb = (msb & ~0xf000) | 0x1000; // v1
+            UUID uuid = new UUID(msb, lsb);
+            assertThat(uuid.version()).isEqualTo(1);
+            UUIDBasedSSTableUniqueIdentifier id = new 
UUIDBasedSSTableUniqueIdentifier(uuid);
+            ByteBuffer buf = id.asBytes();
+            UUIDBasedSSTableUniqueIdentifier fromBytes = 
UUIDBasedSSTableUniqueIdentifier.Builder.instance.fromBytes(buf);
+            assertThat(fromBytes).isEqualTo(id);
+            String s = id.asString();
+            assertThat(s).hasSize(UUIDBasedSSTableUniqueIdentifier.STRING_LEN);
+            
assertThat(s).matches(Pattern.compile("[0-9a-z]{4}_[0-9a-z]{4}_[0-9a-z]{18}"));
+            UUIDBasedSSTableUniqueIdentifier fromString = 
UUIDBasedSSTableUniqueIdentifier.Builder.instance.fromString(s);
+            assertThat(fromString).isEqualTo(id);
+        });

Review comment:
       The test is a bit hard to read. I would split it in to sub-parts like 
`testBytesSerialization` and `testStringSerialization`.
   It would also be good to check that the underlying UUID is still equals to 
the original one.

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SequenceBasedSSTableUniqueIdentifier.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Generation identifier based on sequence of integers.
+ * This has been the standard implementation in C* since inception.
+ */
+public class SequenceBasedSSTableUniqueIdentifier implements 
SSTableUniqueIdentifier
+{
+    public final int generation;
+
+    public SequenceBasedSSTableUniqueIdentifier(final int generation)
+    {
+        assert generation >= 0;
+
+        this.generation = generation;
+    }
+
+    @Override
+    public int compareTo(SSTableUniqueIdentifier o)
+    {
+        if (o instanceof SequenceBasedSSTableUniqueIdentifier)
+            return Integer.compare(this.generation, 
((SequenceBasedSSTableUniqueIdentifier) o).generation);
+
+        return -1;
+    }

Review comment:
       I am not convinced that comparing different types of 
`SSTableUniqueIdentifier` within each sub-class is a safe approach.
   I would support only comparison with its own type and have a comparator that 
take care of sorting accross all the sub-types. That would centralise the logic 
in one place and would make things less error prone if we ever add more 
sub-types.

##########
File path: src/java/org/apache/cassandra/io/sstable/SSTableUniqueIdentifier.java
##########
@@ -0,0 +1,78 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+/**
+ * Represents a unique identifier in the sstable descriptor filename.
+ * This ensures each sstable files uniqueness in the system.

Review comment:
       The term system is pretty vague. I think that we should be precise in 
the level of uniqueness we expect: globaly, cluster-wide ... 

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SequenceBasedSSTableUniqueIdentifier.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Generation identifier based on sequence of integers.
+ * This has been the standard implementation in C* since inception.
+ */
+public class SequenceBasedSSTableUniqueIdentifier implements 
SSTableUniqueIdentifier
+{
+    public final int generation;
+
+    public SequenceBasedSSTableUniqueIdentifier(final int generation)
+    {
+        assert generation >= 0;
+
+        this.generation = generation;
+    }
+
+    @Override
+    public int compareTo(SSTableUniqueIdentifier o)
+    {
+        if (o instanceof SequenceBasedSSTableUniqueIdentifier)
+            return Integer.compare(this.generation, 
((SequenceBasedSSTableUniqueIdentifier) o).generation);
+
+        return -1;
+    }
+
+    @Override
+    public boolean equals(Object o)
+    {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        SequenceBasedSSTableUniqueIdentifier that = 
(SequenceBasedSSTableUniqueIdentifier) o;
+        return generation == that.generation;
+    }
+
+    @Override
+    public int hashCode()
+    {
+        return Objects.hash(generation);
+    }
+
+    @Override
+    public ByteBuffer asBytes()
+    {
+        ByteBuffer bytes = ByteBuffer.allocate(Integer.BYTES);
+        bytes.putInt(0, generation);
+        return bytes;
+    }
+
+    @Override
+    public String asString()
+    {
+        return String.valueOf(generation);
+    }
+
+    @Override
+    public String toString()
+    {
+        return asString();
+    }
+
+    public static class Builder implements 
SSTableUniqueIdentifier.Builder<SequenceBasedSSTableUniqueIdentifier>
+    {
+        public final static Builder instance = new Builder();
+
+        /**
+         * Generates a sequential number to represent an sstables 
'generation'. The first generated identifier will be
+         * greater by one than the largest generation number found across the 
provided existing identifiers.
+         */
+        @Override
+        public Supplier<SequenceBasedSSTableUniqueIdentifier> 
generator(Stream<SSTableUniqueIdentifier> existingIdentifiers)
+        {
+            int value = 
existingIdentifiers.filter(SequenceBasedSSTableUniqueIdentifier.class::isInstance)
+                                           
.map(SequenceBasedSSTableUniqueIdentifier.class::cast)
+                                           .mapToInt(id -> id.generation)
+                                           .max()
+                                           .orElse(0);
+
+            AtomicInteger fileIndexGenerator = new AtomicInteger(value);
+            return () -> new 
SequenceBasedSSTableUniqueIdentifier(fileIndexGenerator.incrementAndGet());
+        }
+
+        @Override
+        public SequenceBasedSSTableUniqueIdentifier fromString(String token) 
throws IllegalArgumentException
+        {
+            return new 
SequenceBasedSSTableUniqueIdentifier(Integer.parseInt(token));
+        }
+
+        @Override
+        public SequenceBasedSSTableUniqueIdentifier fromBytes(ByteBuffer 
bytes) throws IllegalArgumentException
+        {
+            Preconditions.checkArgument(bytes.remaining() >= Integer.BYTES, 
"Buffer does not have enough data: %s", bytes.remaining());

Review comment:
       May be we should use `Buffer does not have enough bytes remaining. 
Expecting: 8 but was : %s`

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SSTableUniqueIdentifierFactory.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+
+import org.apache.cassandra.config.DatabaseDescriptor;
+
+public class SSTableUniqueIdentifierFactory
+{
+    public static final SSTableUniqueIdentifierFactory instance = new 
SSTableUniqueIdentifierFactory();
+
+    /**
+     * Tries to guess the exact identifier type and create an instance of 
{@link SSTableUniqueIdentifier} using
+     * {@link SSTableUniqueIdentifier.Builder#fromString(String)} from the 
guessed builder

Review comment:
       The comment is a bit scary. I would expect the method to return the 
proper identifier. What if it guess wrongly? 😰

##########
File path: 
src/java/org/apache/cassandra/io/sstable/UUIDBasedSSTableUniqueIdentifier.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.Objects;
+import java.util.UUID;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+import javax.annotation.Nonnull;
+
+import com.google.common.base.Preconditions;
+
+import org.apache.cassandra.utils.UUIDGen;
+
+/**
+ * SSTable generation identifiers that can be stored across nodes in one 
directory/bucket
+ * <p>
+ * Uses the UUID v1 identifiers
+ */
+public final class UUIDBasedSSTableUniqueIdentifier implements 
SSTableUniqueIdentifier
+{
+    public final static int STRING_LEN = 28;
+    public final static int BYTES_LEN = 16;
+
+    private final UUID uuid;
+
+    public UUIDBasedSSTableUniqueIdentifier(UUID uuid)
+    {
+        Preconditions.checkArgument(uuid.version() == 1, "%s", uuid.version());
+        this.uuid = uuid;
+    }
+
+    @Override
+    public ByteBuffer asBytes()
+    {
+        return ByteBuffer.allocate(16)
+                         .order(ByteOrder.BIG_ENDIAN)
+                         .putLong(0, uuid.timestamp())
+                         .putLong(Long.BYTES, uuid.getLeastSignificantBits());
+    }
+
+    @Override
+    public String asString()
+    {
+        long ts = uuid.timestamp();
+        long nanoPart = ts % 10_000_000;
+        ts = ts / 10_000_000;
+        long seconds = ts % 86_400;
+        ts = ts / 86_400;
+
+        return String.format("%4s_%4s_%5s%13s",
+                             Long.toString(ts, 36),
+                             Long.toString(seconds, 36),
+                             Long.toString(nanoPart, 36),
+                             
Long.toUnsignedString(uuid.getLeastSignificantBits(), 36)).replace(' ', '0');
+    }
+
+    @Override
+    public String toString()
+    {
+        return uuid.toString();
+    }
+
+    @Override
+    public int compareTo(SSTableUniqueIdentifier o)
+    {
+        //Assumed sstables with a diff identifier are old
+        return o instanceof UUIDBasedSSTableUniqueIdentifier ? 
uuid.compareTo(((UUIDBasedSSTableUniqueIdentifier) o).uuid) : +1;
+    }
+
+    @Override
+    public boolean equals(Object o)
+    {
+
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;

Review comment:
       nit: formatting

##########
File path: NEWS.txt
##########
@@ -64,6 +64,10 @@ New features
       than max_window_in_ms. If there is, a hint is not persisted. If there is 
not, it is.
       This behaviour might be reverted as it was in previous version by 
property hint_window_persistent_enabled by
       setting it to false. This property is by default set to true.
+    - Starting from 4.1 sstables support UUID based generation identifiers. 
They are disabled by default in cassandra.yaml
+      because once enabled, there is no easy way to downgrade. When the node 
is restarted with UUID based generation
+      identifiers enabled, each newly created sstable will have UUID based 
generation identifier and such files are
+      not readable by previous Cassandra versions. In the future those new 
identifiers will become enabled by default.

Review comment:
       Currently we are simply saying that a user can switch to UUID based 
generation identifiers. I doubt that the benefits of this feature will be 
obvious for most people. We should brought forward what are the advantages of 
that change.

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SSTableUniqueIdentifierFactory.java
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+
+import org.apache.cassandra.config.DatabaseDescriptor;
+
+public class SSTableUniqueIdentifierFactory
+{
+    public static final SSTableUniqueIdentifierFactory instance = new 
SSTableUniqueIdentifierFactory();
+
+    /**
+     * Tries to guess the exact identifier type and create an instance of 
{@link SSTableUniqueIdentifier} using
+     * {@link SSTableUniqueIdentifier.Builder#fromString(String)} from the 
guessed builder
+     *
+     * @throws IllegalArgumentException when the provided string 
representation is malformed
+     */
+    public SSTableUniqueIdentifier fromString(String str) throws 
IllegalArgumentException
+    {
+        SSTableUniqueIdentifier.Builder<?> builder = str.length() == 
UUIDBasedSSTableUniqueIdentifier.STRING_LEN

Review comment:
       May be we should add a `isUniqueIdentifier(String)` method to 
`UUIDBasedSSTableUniqueIdentifier` and make sure that it cannot return `true` 
for an integer large enough.

##########
File path: 
src/java/org/apache/cassandra/io/sstable/SequenceBasedSSTableUniqueIdentifier.java
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.cassandra.io.sstable;
+
+import java.nio.ByteBuffer;
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+import com.google.common.base.Preconditions;
+
+/**
+ * Generation identifier based on sequence of integers.
+ * This has been the standard implementation in C* since inception.
+ */
+public class SequenceBasedSSTableUniqueIdentifier implements 
SSTableUniqueIdentifier
+{
+    public final int generation;
+
+    public SequenceBasedSSTableUniqueIdentifier(final int generation)
+    {
+        assert generation >= 0;
+
+        this.generation = generation;
+    }
+
+    @Override
+    public int compareTo(SSTableUniqueIdentifier o)
+    {
+        if (o instanceof SequenceBasedSSTableUniqueIdentifier)
+            return Integer.compare(this.generation, 
((SequenceBasedSSTableUniqueIdentifier) o).generation);
+
+        return -1;
+    }
+
+    @Override
+    public boolean equals(Object o)
+    {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;

Review comment:
       nit: the return statement should be on a new line 




-- 
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]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to