This is an automated email from the ASF dual-hosted git repository.
blerer pushed a commit to branch cep-21-tcm-review
in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit 509d33155f9e222a086b3d61c969208dad6fe722
Author: Benjamin Lerer <b.le...@gmail.com>
AuthorDate: Tue Oct 17 13:10:15 2023 +0200
Simplify the RecentlySealedPeriod logic
---
.../cassandra/tcm/RecentlySealedPeriods.java | 92 +++++++++-------------
src/java/org/apache/cassandra/tcm/Sealed.java | 2 +-
.../cassandra/tcm/RecentlySealedPeriodsTest.java | 18 +++--
3 files changed, 48 insertions(+), 64 deletions(-)
diff --git a/src/java/org/apache/cassandra/tcm/RecentlySealedPeriods.java
b/src/java/org/apache/cassandra/tcm/RecentlySealedPeriods.java
index cc16ae656e..cca1d7a213 100644
--- a/src/java/org/apache/cassandra/tcm/RecentlySealedPeriods.java
+++ b/src/java/org/apache/cassandra/tcm/RecentlySealedPeriods.java
@@ -18,11 +18,12 @@
package org.apache.cassandra.tcm;
-import java.util.Arrays;
-import java.util.Collections;
import java.util.List;
+import java.util.Map;
+import java.util.NavigableMap;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSortedMap;
import org.apache.cassandra.config.CassandraRelevantProperties;
@@ -34,50 +35,40 @@ import
org.apache.cassandra.config.CassandraRelevantProperties;
* the target is outside the range of this index, we eventually fall back to a
read
* from the system.metadata_sealed_periods table.
*/
-public class RecentlySealedPeriods
+public final class RecentlySealedPeriods
{
- public static final RecentlySealedPeriods EMPTY = new
RecentlySealedPeriods(new Sealed[0]);
+ public static final RecentlySealedPeriods EMPTY = new
RecentlySealedPeriods(ImmutableSortedMap.of());
/**
* The maximum number of sealed periods stored in memory.
*/
- private int maxSize =
CassandraRelevantProperties.TCM_RECENTLY_SEALED_PERIOD_INDEX_SIZE.getInt();
- private Sealed[] recent;
+ private final int maxSize =
CassandraRelevantProperties.TCM_RECENTLY_SEALED_PERIOD_INDEX_SIZE.getInt();
+ private final NavigableMap<Epoch, Sealed> recent;
- private RecentlySealedPeriods(Sealed first)
- {
- this.recent = new Sealed[]{first};
- }
-
- private RecentlySealedPeriods(Sealed[] recent)
+ private RecentlySealedPeriods(ImmutableSortedMap<Epoch, Sealed> recent)
{
this.recent = recent;
}
public static RecentlySealedPeriods init(List<Sealed> recent)
{
- Collections.sort(recent);
- return new RecentlySealedPeriods(recent.toArray(new
Sealed[recent.size()]));
+ ImmutableSortedMap.Builder<Epoch, Sealed> builder =
ImmutableSortedMap.naturalOrder();
+ for (Sealed sealed: recent)
+ {
+ builder.put(sealed.epoch, sealed);
+ }
+ return new RecentlySealedPeriods(builder.build());
}
-
public RecentlySealedPeriods with(Epoch epoch, long period)
{
- if (recent == null)
- {
- return new RecentlySealedPeriods(new Sealed(period, epoch));
- }
- else
- {
- int toCopy = Math.min(recent.length, maxSize - 1);
- int newSize = Math.min(recent.length + 1, maxSize);
- Sealed[] newList = new Sealed[newSize];
- System.arraycopy(recent, recent.length - toCopy, newList, 0,
toCopy);
- newList[newSize - 1] = new Sealed(period, epoch);
- // shouldn't be necessary, but is cheap
- Arrays.sort(newList, Sealed::compareTo);
- return new RecentlySealedPeriods(newList);
- }
+ NavigableMap<Epoch, Sealed> toKeep = recent.size() < maxSize ? recent
+ :
recent.tailMap(recent.firstKey(), false);
+
+ return new RecentlySealedPeriods(ImmutableSortedMap.<Epoch,
Sealed>naturalOrder()
+ .putAll(toKeep)
+ .put(epoch, new
Sealed(period, epoch))
+ .build());
}
/**
@@ -86,28 +77,27 @@ public class RecentlySealedPeriods
* as long as its epoch is greater than the target.
* If the target epoch is greater than the max epoch in the latest sealed
* period, then assume there is no suitable snapshot.
- * @param epoch
+ * @param epoch the target epoch
* @return
*/
public Sealed lookupEpochForSnapshot(Epoch epoch)
{
+ if (recent.isEmpty())
+ return Sealed.EMPTY;
+
// if the target is > the highest indexed value there's no need to
// scan the index. Instead, just signal to the caller that no suitable
// sealed period was found.
- if (recent.length > 0)
- {
- Sealed latest = recent[recent.length - 1];
- return latest.epoch.isAfter(epoch) ? latest : Sealed.EMPTY;
- }
- return Sealed.EMPTY;
+ Map.Entry<Epoch, Sealed> latest = recent.lastEntry();
+ return latest.getKey().isAfter(epoch) ? latest.getValue() :
Sealed.EMPTY;
}
// TODO add a lookupEpochForSnapshotBetween(start, end) so we can walk
// through the index if the latest snapshot happens to be missing
/**
- * Find the *closest* sealed period to a target epoch. Used to identify
- * the period to start at if building a list of all log entries with an
+ * Find the sealed period stricly greater than the target epoch.
+ * <p>This method is used to identify the period to start from if building
a list of all log entries with an
* epoch greater than the one supplied. If the target epoch happens to be
* the max in a sealed period, we would start with the period following
* that. If the target epoch is equal to or after the max in the latest
@@ -117,28 +107,18 @@ public class RecentlySealedPeriods
*/
public Sealed lookupPeriodForReplication(Epoch epoch)
{
- // if the target is > the highest indexed value there's no need to
- // scan the index. Instead, just signal to the caller that no suitable
- // sealed period was found.
- if (recent.length > 0 && epoch.isEqualOrAfter(recent[recent.length -
1].epoch))
- return Sealed.EMPTY;
-
- for (int i = recent.length - 1; i >= 0; i--)
- {
- Sealed e = recent[i];
- if (e.epoch.isEqualOrBefore(epoch))
- return recent[Math.min(i + 1, recent.length - 1)];
- }
-
- // the target epoch couldn't be found in the index, signal to the
+ // if the target is not within the index we need to signal to the
// caller that a slower/more expensive/more comprehensive lookup
// is required
- return Sealed.EMPTY;
+ if (recent.isEmpty() || epoch.isBefore(recent.firstKey()) ||
epoch.isEqualOrAfter(recent.lastKey()))
+ return Sealed.EMPTY;
+
+ return recent.higherEntry(epoch).getValue();
}
@VisibleForTesting
- public Sealed[] array()
+ public Sealed[] toArray()
{
- return recent;
+ return recent.values().toArray(new Sealed[recent.size()]);
}
}
diff --git a/src/java/org/apache/cassandra/tcm/Sealed.java
b/src/java/org/apache/cassandra/tcm/Sealed.java
index 574131b626..2a38d92fc0 100644
--- a/src/java/org/apache/cassandra/tcm/Sealed.java
+++ b/src/java/org/apache/cassandra/tcm/Sealed.java
@@ -150,6 +150,6 @@ public class Sealed implements Comparable<Sealed>
@Override
public int compareTo(Sealed o)
{
- return Long.compare(this.epoch.getEpoch(), o.epoch.getEpoch());
+ return this.epoch.compareTo(o.epoch);
}
}
diff --git a/test/unit/org/apache/cassandra/tcm/RecentlySealedPeriodsTest.java
b/test/unit/org/apache/cassandra/tcm/RecentlySealedPeriodsTest.java
index b0d236a67b..6bc3d6ab4f 100644
--- a/test/unit/org/apache/cassandra/tcm/RecentlySealedPeriodsTest.java
+++ b/test/unit/org/apache/cassandra/tcm/RecentlySealedPeriodsTest.java
@@ -19,6 +19,7 @@
package org.apache.cassandra.tcm;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Random;
@@ -49,11 +50,14 @@ public class RecentlySealedPeriodsTest
for (int i=1; i<=10;i++)
idx = idx.with(Epoch.create((i*3)+1), i);
- assertEquals(10, idx.array().length);
+ System.out.println(Arrays.toString(idx.toArray()));
+
+
+ assertEquals(10, idx.toArray().length);
// lowest indexed period is 1
- assertEquals(1, idx.array()[0].period);
+ assertEquals(1, idx.toArray()[0].period);
// greatest indexed period is 10
- assertEquals(10, idx.array()[idx.array().length - 1].period);
+ assertEquals(10, idx.toArray()[idx.toArray().length - 1].period);
assertEquals(7,
idx.lookupPeriodForReplication(Epoch.create(20)).period);
// looking an epoch which is the max in a sealed period, should return
the next period (for replication purposes)
@@ -68,11 +72,11 @@ public class RecentlySealedPeriodsTest
// add an additional entry
idx = idx.with(Epoch.create(100), 11);
// reassert the boundaries
- assertEquals(10, idx.array().length);
+ assertEquals(10, idx.toArray().length);
// lowest indexed period is now 2
- assertEquals(2, idx.array()[0].period);
+ assertEquals(2, idx.toArray()[0].period);
// greatest indexed period is now 11
- assertEquals(11, idx.array()[idx.array().length - 1].period);
+ assertEquals(11, idx.toArray()[idx.toArray().length - 1].period);
// lookup an epoch within the (new) maximum sealed period
assertEquals(11,
idx.lookupPeriodForReplication(Epoch.create(76)).period);
@@ -101,7 +105,7 @@ public class RecentlySealedPeriodsTest
for (Long epoch : keys)
idx = idx.with(Epoch.create(epoch), epoch/10);
- Sealed[] index = idx.array();
+ Sealed[] index = idx.toArray();
for(int i=0; i<index.length; i++)
{
Sealed entry = index[i];
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
