pauloricardomg commented on code in PR #3374:
URL: https://github.com/apache/cassandra/pull/3374#discussion_r1759806037
##########
src/java/org/apache/cassandra/config/CassandraRelevantProperties.java:
##########
@@ -490,6 +490,12 @@ public enum CassandraRelevantProperties
SNAPSHOT_CLEANUP_INITIAL_DELAY_SECONDS("cassandra.snapshot.ttl_cleanup_initial_delay_seconds",
"5"),
/** snapshots ttl cleanup period in seconds */
SNAPSHOT_CLEANUP_PERIOD_SECONDS("cassandra.snapshot.ttl_cleanup_period_seconds",
"60"),
+ /**
+ * When there is a snapshot with old / basic format (basically
pre-CASSANDRA-16789),
+ * it will enrich it with more metadata upon snapshot's loading at startup.
+ * Defaults to true, when set to false, no enriching will be done.
+ * */
+ SNAPSHOT_MANIFEST_ENRICH_ENABLED("cassandra.snapshot.enrich.enabled",
"true"),
Review Comment:
Since this is an invasive refactoring/cleanup patch I'd prefer to leave this
behavior change to another ticket, unless this is directly related to caching
and centralizing snapshots in SnapshotManager (CASSANDRA-18111).
##########
src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java:
##########
@@ -17,156 +17,830 @@
*/
package org.apache.cassandra.service.snapshot;
-
+import java.io.IOException;
+import java.io.PrintStream;
+import java.time.Instant;
+import java.time.format.DateTimeParseException;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.PriorityQueue;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.PriorityBlockingQueue;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+import javax.management.openmbean.TabularData;
+import javax.management.openmbean.TabularDataSupport;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.RateLimiter;
+import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.concurrent.ScheduledExecutorPlus;
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.config.DurationSpec;
+import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Directories;
-
-import java.util.concurrent.TimeoutException;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
-
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.db.SchemaCQLHelper;
+import org.apache.cassandra.db.SnapshotDetailsTabularData;
+import org.apache.cassandra.db.lifecycle.SSTableSet;
+import org.apache.cassandra.db.lifecycle.View;
+import org.apache.cassandra.io.FSWriteError;
+import org.apache.cassandra.io.sstable.format.SSTableFormat;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.File;
+import org.apache.cassandra.io.util.FileOutputStreamPlus;
+import org.apache.cassandra.io.util.FileUtils;
+import org.apache.cassandra.schema.SchemaConstants;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.Clock;
import org.apache.cassandra.utils.ExecutorUtils;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.cassandra.utils.MBeanWrapper;
+import static java.lang.String.format;
import static java.util.Comparator.comparing;
-import static java.util.stream.Collectors.toList;
+import static java.util.concurrent.TimeUnit.SECONDS;
import static
org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory;
+import static
org.apache.cassandra.schema.SchemaConstants.isLocalSystemKeyspace;
import static org.apache.cassandra.utils.FBUtilities.now;
-public class SnapshotManager {
+public class SnapshotManager implements SnapshotManagerMBean, AutoCloseable
+{
+ private static final Logger logger =
LoggerFactory.getLogger(SnapshotManager.class);
private static final ScheduledExecutorPlus executor =
executorFactory().scheduled(false, "SnapshotCleanup");
- private static final Logger logger =
LoggerFactory.getLogger(SnapshotManager.class);
+ public static final SnapshotManager instance = new SnapshotManager();
private final long initialDelaySeconds;
private final long cleanupPeriodSeconds;
private final SnapshotLoader snapshotLoader;
+ public final RateLimiter snapshotRateLimiter;
- @VisibleForTesting
- protected volatile ScheduledFuture<?> cleanupTaskFuture;
+ private volatile ScheduledFuture<?> cleanupTaskFuture;
+
+ private final Set<TableSnapshot> liveSnapshots =
Collections.synchronizedSet(new HashSet<>());
/**
* Expiring snapshots ordered by expiration date, to allow only iterating
over snapshots
- * that need to be removed on {@link this#clearExpiredSnapshots()}
+ * that need to be removed
*/
- private final PriorityQueue<TableSnapshot> expiringSnapshots = new
PriorityQueue<>(comparing(TableSnapshot::getExpiresAt));
+ private final PriorityBlockingQueue<TableSnapshot> expiringSnapshots = new
PriorityBlockingQueue<>(10, comparing(TableSnapshot::getExpiresAt));
- public SnapshotManager()
+ private SnapshotManager()
{
this(CassandraRelevantProperties.SNAPSHOT_CLEANUP_INITIAL_DELAY_SECONDS.getInt(),
CassandraRelevantProperties.SNAPSHOT_CLEANUP_PERIOD_SECONDS.getInt());
}
@VisibleForTesting
protected SnapshotManager(long initialDelaySeconds, long
cleanupPeriodSeconds)
+ {
+ this(initialDelaySeconds, cleanupPeriodSeconds,
DatabaseDescriptor.getAllDataFileLocations());
+ }
+
+ @VisibleForTesting
+ protected SnapshotManager(long initialDelaySeconds, long
cleanupPeriodSeconds, String[] dataDirs)
{
this.initialDelaySeconds = initialDelaySeconds;
this.cleanupPeriodSeconds = cleanupPeriodSeconds;
- snapshotLoader = new
SnapshotLoader(DatabaseDescriptor.getAllDataFileLocations());
+ snapshotLoader = new SnapshotLoader(dataDirs);
+ snapshotRateLimiter = DatabaseDescriptor.getSnapshotRateLimiter();
}
- public Collection<TableSnapshot> getExpiringSnapshots()
+ public void registerMBean()
{
- return expiringSnapshots;
+ logger.debug("Registering SnapshotManagerMBean");
+ MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
}
- public synchronized void start()
+ public void unregisterMBean()
+ {
+ MBeanWrapper.instance.unregisterMBean(MBEAN_NAME);
+ }
+
+ public static void shutdownAndWait(long timeout, TimeUnit unit) throws
InterruptedException, TimeoutException
+ {
+ ExecutorUtils.shutdownNowAndWait(timeout, unit, executor);
+ }
+
+ public synchronized void start(boolean runPeriodicSnapshotCleaner)
{
addSnapshots(loadSnapshots());
- resumeSnapshotCleanup();
+ if (runPeriodicSnapshotCleaner)
+ resumeSnapshotCleanup();
+ }
+
+ public synchronized void start()
+ {
+ start(false);
}
- public synchronized void stop() throws InterruptedException,
TimeoutException
+ @Override
+ public synchronized void close()
{
+ pauseSnapshotCleanup();
expiringSnapshots.clear();
+ liveSnapshots.clear();
+ }
+
+ public synchronized void close(boolean shutdownExecutor) throws Exception
+ {
+ close();
+ if (shutdownExecutor)
+ shutdownAndWait(1, TimeUnit.MINUTES);
+ }
+
+ public synchronized Set<TableSnapshot> loadSnapshots()
+ {
+ return snapshotLoader.loadSnapshots();
+ }
+
+ public synchronized void restart()
+ {
+ restart(true);
+ }
+
+ public synchronized void restart(boolean runPeriodicSnapshotCleaner)
+ {
+ logger.debug("Restarting SnapshotManager");
+ close();
+ start(runPeriodicSnapshotCleaner);
+ logger.debug("SnapshotManager restarted");
+ }
+
+ synchronized void addSnapshot(TableSnapshot snapshot)
+ {
+ logger.debug("Adding snapshot {}", snapshot);
+
+ if (snapshot.isExpiring())
+ expiringSnapshots.add(snapshot);
+ else
+ liveSnapshots.add(snapshot);
+ }
+
+ synchronized void addSnapshots(Collection<TableSnapshot> snapshots)
+ {
+ snapshots.forEach(this::addSnapshot);
+ }
+
+ public synchronized void resumeSnapshotCleanup()
+ {
+ if (cleanupTaskFuture == null)
+ {
+ logger.info("Scheduling expired snapshots cleanup with
initialDelaySeconds={} and cleanupPeriodSeconds={}",
+ initialDelaySeconds, cleanupPeriodSeconds);
+
+ cleanupTaskFuture =
executor.scheduleWithFixedDelay(this::clearExpiredSnapshots,
+
initialDelaySeconds,
+
cleanupPeriodSeconds,
+ SECONDS);
+ }
+ }
+
+ synchronized void pauseSnapshotCleanup()
+ {
if (cleanupTaskFuture != null)
{
cleanupTaskFuture.cancel(false);
cleanupTaskFuture = null;
}
}
- public synchronized void addSnapshot(TableSnapshot snapshot)
+ /**
+ * Deletes snapshot and removes it from manager.
+ *
+ * @param snapshot snapshot to clear
+ */
+ synchronized void clearSnapshot(TableSnapshot snapshot)
{
- // We currently only care about expiring snapshots
- if (snapshot.isExpiring())
+ clearSnapshot(snapshot, true);
+ }
+
+ synchronized void clearSnapshot(TableSnapshot snapshot, boolean deleteData)
+ {
+ logger.debug("Removing snapshot {}{}", snapshot, deleteData ? ",
deleting data" : "");
+
+ if (deleteData)
{
- logger.debug("Adding expiring snapshot {}", snapshot);
- expiringSnapshots.add(snapshot);
+ for (File snapshotDir : snapshot.getDirectories())
+ {
+ try
+ {
+ removeSnapshotDirectory(snapshotDir);
+ }
+ catch (Exception ex)
+ {
+ logger.warn("Unable to remove snapshot directory {}",
snapshotDir, ex);
+ }
+ }
}
+
+ if (snapshot.isExpiring())
+ expiringSnapshots.remove(snapshot);
+ else
+ liveSnapshots.remove(snapshot);
}
- public synchronized Set<TableSnapshot> loadSnapshots(String keyspace)
+ /**
+ * Returns list of snapshots of given keyspace
+ *
+ * @param keyspace keyspace of a snapshot
+ * @return list of snapshots of given keyspace.
+ */
+ public List<TableSnapshot> getSnapshots(String keyspace)
{
- return snapshotLoader.loadSnapshots(keyspace);
+ return getSnapshots(snapshot ->
snapshot.getKeyspaceName().equals(keyspace));
}
- public synchronized Set<TableSnapshot> loadSnapshots()
+ /**
+ * Returns list of snapshots from given keyspace and table.
+ *
+ * @param keyspace keyspace of a snapshot
+ * @param table table of a snapshot
+ * @return list of snapshots from given keyspace and table
+ */
+ public List<TableSnapshot> getSnapshots(String keyspace, String table)
{
- return snapshotLoader.loadSnapshots();
+ return getSnapshots(snapshot ->
snapshot.getKeyspaceName().equals(keyspace) &&
+ snapshot.getTableName().equals(table));
}
- @VisibleForTesting
- protected synchronized void addSnapshots(Collection<TableSnapshot>
snapshots)
+ /**
+ * Returns a snapshot or empty optional based on the given parameters.
+ *
+ * @param keyspace keyspace of a snapshot
+ * @param table table of a snapshot
+ * @param tag name of a snapshot
+ * @return empty optional if there is not such snapshot, non-empty
otherwise
+ */
+ public synchronized Optional<TableSnapshot> getSnapshot(String keyspace,
String table, String tag)
{
- logger.debug("Adding snapshots: {}.", Joiner.on(",
").join(snapshots.stream().map(TableSnapshot::getId).collect(toList())));
- snapshots.forEach(this::addSnapshot);
+ // we do not use the predicate here because we want to stop the loop
as soon as
+ // we find the snapshot we are looking for, looping until the end is
not necessary
+ for (TableSnapshot snapshot : Iterables.concat(liveSnapshots,
expiringSnapshots))
+ {
+ if (snapshot.getKeyspaceName().equals(keyspace) &&
+ snapshot.getTableName().equals(table) &&
+ snapshot.getTag().equals(tag) || (tag != null &&
tag.isEmpty()))
+ {
+ return Optional.of(snapshot);
+ }
+ }
+
+ return Optional.empty();
}
- // TODO: Support pausing snapshot cleanup
- @VisibleForTesting
- synchronized void resumeSnapshotCleanup()
+ /**
+ * Return snapshots based on given parameters.
+ *
+ * @param skipExpiring if expiring snapshots should be skipped
+ * @param includeEphemeral if ephemeral snapshots should be included
+ * @return snapshots based on given parameters
+ */
+ public List<TableSnapshot> getSnapshots(boolean skipExpiring, boolean
includeEphemeral)
{
- if (cleanupTaskFuture == null)
+ return getSnapshots(s -> (!skipExpiring || !s.isExpiring()) &&
(includeEphemeral || !s.isEphemeral()));
+ }
+
+ /**
+ * @return all ephemeral snapshots in a node
+ */
+ public List<TableSnapshot> getEphemeralSnapshots()
+ {
+ return getSnapshots(TableSnapshot::isEphemeral);
+ }
+
+ /**
+ * Returns all snapshots passing the given predicate.
+ *
+ * @param predicate predicate to filter all snapshots of
+ * @return list of snapshots passing the predicate
+ */
+ public synchronized List<TableSnapshot>
getSnapshots(Predicate<TableSnapshot> predicate)
+ {
+ List<TableSnapshot> notExistingAnymore = new ArrayList<>();
+ List<TableSnapshot> snapshots = new ArrayList<>();
+ for (TableSnapshot snapshot : Iterables.concat(liveSnapshots,
expiringSnapshots))
{
- logger.info("Scheduling expired snapshot cleanup with
initialDelaySeconds={} and cleanupPeriodSeconds={}",
- initialDelaySeconds, cleanupPeriodSeconds);
- cleanupTaskFuture =
executor.scheduleWithFixedDelay(this::clearExpiredSnapshots,
initialDelaySeconds,
-
cleanupPeriodSeconds, TimeUnit.SECONDS);
+ if (predicate.test(snapshot))
+ {
+ if (!snapshot.hasManifest())
+ notExistingAnymore.add(snapshot);
+ else
+ snapshots.add(snapshot);
+ }
}
+
+ for (TableSnapshot tableSnapshot : notExistingAnymore)
+ clearSnapshot(tableSnapshot, false);
+
+ return snapshots;
}
- @VisibleForTesting
- protected synchronized void clearExpiredSnapshots()
+ public Collection<TableSnapshot> getExpiringSnapshots()
+ {
+ return expiringSnapshots;
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspaces.
+ * <p>
+ * If tag is not present / is empty, all snapshots are considered to be
cleared.
+ * If keyspaces are empty, all snapshots of given tag and older than
maxCreatedAt are removed.
+ * <p>
+ * Ephemeral snapshots are not included.
+ *
+ * @param tag optional tag of snapshot to clear
+ * @param keyspaces keyspaces to remove snapshots for
+ * @param maxCreatedAt clear all such snapshots which were created before
this timestamp
+ */
+ public void clearSnapshots(String tag, Set<String> keyspaces, long
maxCreatedAt)
+ {
+ clearSnapshots(tag, keyspaces, maxCreatedAt, false);
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspace.
+ * <p>
+ *
+ * @param tag snapshot name
+ * @param keyspace keyspace to clear all snapshots of a given tag of
+ */
+ public void clearSnapshots(String tag, String keyspace)
+ {
+ clearSnapshots(tag, Set.of(keyspace),
Clock.Global.currentTimeMillis(), false);
+ }
+
+ /**
+ * Removes a snapshot.
+ * <p>
+ *
+ * @param keyspace keyspace of a snapshot to remove
+ * @param table table of a snapshot to remove
+ * @param tag name of a snapshot to remove.
+ */
+ public void clearSnapshot(String keyspace, String table, String tag)
+ {
+ getSnapshot(keyspace, table, tag).ifPresent(this::clearSnapshot);
+ }
+
+ /**
+ * Clears all ephemeral snapshots in a node.
+ */
+ public void clearEphemeralSnapshots()
{
- TableSnapshot expiredSnapshot;
- while ((expiredSnapshot = expiringSnapshots.peek()) != null)
+ getEphemeralSnapshots().forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Clears all expired snapshots in a node.
+ */
+ public synchronized void clearExpiredSnapshots()
+ {
+ Instant now = FBUtilities.now();
+ getSnapshots(s -> s.isExpired(now)).forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspaces.
+ * <p>
+ * If tag is not present / is empty, all snapshots are considered to be
cleared.
+ * If keyspaces are empty, all snapshots of given tag and older than
maxCreatedAt are removed.
+ *
+ * @param tag optional tag of snapshot to clear
+ * @param keyspaces keyspaces to remove snapshots for
+ * @param maxCreatedAt clear all such snapshots which were created
before this timestamp
+ * @param includeEphemeral include ephemeral snaphots for removal or not
+ */
+ synchronized void clearSnapshots(String tag, Set<String> keyspaces,
+ long maxCreatedAt,
+ boolean includeEphemeral)
+ {
+ Predicate<TableSnapshot> predicate = shouldClearSnapshot(tag,
keyspaces, maxCreatedAt, includeEphemeral);
+ getSnapshots(predicate).forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Returns a predicate based on which a snapshot will be included for
deletion or not.
+ *
+ * @param tag name of snapshot to remove
+ * @param keyspaces keyspaces this snapshot belongs to
+ * @param olderThanTimestamp clear the snapshot if it is older than given
timestamp
+ * @param includeEphemeral whether to include ephemeral snapshots as well
+ * @return predicate which filters snapshots on given parameters
+ */
+ static Predicate<TableSnapshot> shouldClearSnapshot(String tag,
+ Set<String> keyspaces,
+ long
olderThanTimestamp,
+ boolean
includeEphemeral)
+ {
+ return ts ->
{
- if (!expiredSnapshot.isExpired(now()))
- break; // the earliest expiring snapshot is not expired yet,
so there is no more expired snapshots to remove
+ // When no tag is supplied, all snapshots must be cleared
+ boolean clearAll = tag == null || tag.isEmpty();
+ if (!clearAll && ts.isEphemeral() && !includeEphemeral)
+ logger.info("Skipping deletion of ephemeral snapshot '{}' in
keyspace {}. " +
+ "Ephemeral snapshots are not removable by a user.",
+ tag, ts.getKeyspaceName());
+ boolean passedEphemeralTest = !ts.isEphemeral() ||
(ts.isEphemeral() && includeEphemeral);
+ boolean shouldClearTag = clearAll || ts.getTag().equals(tag);
+ boolean byTimestamp = true;
- logger.debug("Removing expired snapshot {}.", expiredSnapshot);
- clearSnapshot(expiredSnapshot);
- }
+ if (olderThanTimestamp > 0L)
+ {
+ Instant createdAt = ts.getCreatedAt();
+ if (createdAt != null)
+ byTimestamp =
createdAt.isBefore(Instant.ofEpochMilli(olderThanTimestamp));
+ }
+
+ boolean byKeyspace = (keyspaces.isEmpty() ||
keyspaces.contains(ts.getKeyspaceName()));
+
+ return passedEphemeralTest && shouldClearTag && byTimestamp &&
byKeyspace;
+ };
}
/**
- * Deletes snapshot and remove it from manager
+ * Takes a snapshot by creating hardlinks into snapshot directories. This
method also
+ * creates manifests and schema files and such snapshot will be added
among tracked ones in this manager.
+ *
+ * @param cfs column family to create a snapshot for
+ * @param tag name of snapshot
+ * @param ephemeral true if the snapshot is ephemeral, false otherwise
+ * @param ttl time after the created snapshot will be removed
+ * @param creationTime time the snapshot was created
+ * @param rateLimiter limiter for hard-links creation, if null, limiter
from DatabaseDescriptor will be used
+ * @return logical representation of a snapshot
*/
- public synchronized void clearSnapshot(TableSnapshot snapshot)
+ public TableSnapshot createSnapshot(ColumnFamilyStore cfs,
Review Comment:
One of the goals of centralizing snapshot logic into `SnapshotManager` was
to simplify things but this snapshot creation logic is getting complex enough
that probably deserves its own class (ie. `CreateSnapshotTask`) that receives a
set of snapshot creation options (ie. tag/ttl/etc) and returns a
`Set<TableSnapshot>` that is then added to the set of `liveSnapshots`.
This would remove the coupling of `SnapshotManager` with `ColumnFamilyStore`
since all the handling of snapshot creation would be encapsulated in the
`CreateSnapshotTask`.
This would make `SnapshotManager` be responsible for the management of
snapshot lifecycle (similar to `CompactionManager`) while the actual snapshot
creation task would be encapsulated in its own `CreateSnapshotTask` class
(similar to `AbstractCompactionTask`). This is analogous to encapsulating
snapshot listing logic into its own `SnapshotLoader` class to make
`SnapshotManager` leaner.
With this the implementation of `SnapshotManagerMBean.takeSnapshot` would be
something like:
```java
synchronized takeSnapshot(String tag, Map<String, String> options,
String... entities)
{
CreateSnapshotTask task = new CreateSnapshotTask(tag, entitites,
snapshotRateLimiter);
Set<TableSnapshot> snapshot = task.executeBlocking()
liveSnapshots.addAll(snapshot)
}
```
Do you think this makes sense ?
##########
src/java/org/apache/cassandra/db/ColumnFamilyStore.java:
##########
@@ -2368,19 +2224,63 @@ public boolean snapshotExists(String snapshotName)
*/
public void clearSnapshot(String snapshotName)
{
- RateLimiter clearSnapshotRateLimiter =
DatabaseDescriptor.getSnapshotRateLimiter();
-
- List<File> snapshotDirs = getDirectories().getCFDirectories();
- Directories.clearSnapshot(snapshotName, snapshotDirs,
clearSnapshotRateLimiter);
+ SnapshotManager.instance.clearSnapshot(keyspace.getName(),
getTableName(), snapshotName);
}
+
/**
*
* @return Return a map of all snapshots to space being used
* The pair for a snapshot has true size and size on disk.
*/
public Map<String, TableSnapshot> listSnapshots()
{
- return getDirectories().listSnapshots();
+ Set<TableSnapshot> snapshots = new
SnapshotLoader(getDirectories()).loadSnapshots();
+ Map<String, TableSnapshot> tagSnapshotsMap = new HashMap<>();
+
+ for (TableSnapshot snapshot : snapshots)
+ tagSnapshotsMap.put(snapshot.getTag(), snapshot);
+
+ return tagSnapshotsMap;
+ }
+
+ public Refs<SSTableReader> getSnapshotSSTableReaders(String tag) throws
IOException
Review Comment:
Can this be moved to `TableSnapshot` ? I don't think `ColumnFamilyStore`
needs to be involved in how snapshot sstables are read.
##########
src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java:
##########
@@ -17,156 +17,830 @@
*/
package org.apache.cassandra.service.snapshot;
-
+import java.io.IOException;
+import java.io.PrintStream;
+import java.time.Instant;
+import java.time.format.DateTimeParseException;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.PriorityQueue;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
import java.util.Set;
+import java.util.concurrent.PriorityBlockingQueue;
import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.function.Predicate;
+import javax.management.openmbean.TabularData;
+import javax.management.openmbean.TabularDataSupport;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.Iterables;
+import com.google.common.util.concurrent.RateLimiter;
+import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.apache.cassandra.concurrent.ScheduledExecutorPlus;
import org.apache.cassandra.config.CassandraRelevantProperties;
import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.config.DurationSpec;
+import org.apache.cassandra.db.ColumnFamilyStore;
import org.apache.cassandra.db.Directories;
-
-import java.util.concurrent.TimeoutException;
-
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Joiner;
-
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.db.SchemaCQLHelper;
+import org.apache.cassandra.db.SnapshotDetailsTabularData;
+import org.apache.cassandra.db.lifecycle.SSTableSet;
+import org.apache.cassandra.db.lifecycle.View;
+import org.apache.cassandra.io.FSWriteError;
+import org.apache.cassandra.io.sstable.format.SSTableFormat;
+import org.apache.cassandra.io.sstable.format.SSTableReader;
import org.apache.cassandra.io.util.File;
+import org.apache.cassandra.io.util.FileOutputStreamPlus;
+import org.apache.cassandra.io.util.FileUtils;
+import org.apache.cassandra.schema.SchemaConstants;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.utils.Clock;
import org.apache.cassandra.utils.ExecutorUtils;
+import org.apache.cassandra.utils.FBUtilities;
+import org.apache.cassandra.utils.MBeanWrapper;
+import static java.lang.String.format;
import static java.util.Comparator.comparing;
-import static java.util.stream.Collectors.toList;
+import static java.util.concurrent.TimeUnit.SECONDS;
import static
org.apache.cassandra.concurrent.ExecutorFactory.Global.executorFactory;
+import static
org.apache.cassandra.schema.SchemaConstants.isLocalSystemKeyspace;
import static org.apache.cassandra.utils.FBUtilities.now;
-public class SnapshotManager {
+public class SnapshotManager implements SnapshotManagerMBean, AutoCloseable
+{
+ private static final Logger logger =
LoggerFactory.getLogger(SnapshotManager.class);
private static final ScheduledExecutorPlus executor =
executorFactory().scheduled(false, "SnapshotCleanup");
- private static final Logger logger =
LoggerFactory.getLogger(SnapshotManager.class);
+ public static final SnapshotManager instance = new SnapshotManager();
private final long initialDelaySeconds;
private final long cleanupPeriodSeconds;
private final SnapshotLoader snapshotLoader;
+ public final RateLimiter snapshotRateLimiter;
- @VisibleForTesting
- protected volatile ScheduledFuture<?> cleanupTaskFuture;
+ private volatile ScheduledFuture<?> cleanupTaskFuture;
+
+ private final Set<TableSnapshot> liveSnapshots =
Collections.synchronizedSet(new HashSet<>());
/**
* Expiring snapshots ordered by expiration date, to allow only iterating
over snapshots
- * that need to be removed on {@link this#clearExpiredSnapshots()}
+ * that need to be removed
*/
- private final PriorityQueue<TableSnapshot> expiringSnapshots = new
PriorityQueue<>(comparing(TableSnapshot::getExpiresAt));
+ private final PriorityBlockingQueue<TableSnapshot> expiringSnapshots = new
PriorityBlockingQueue<>(10, comparing(TableSnapshot::getExpiresAt));
- public SnapshotManager()
+ private SnapshotManager()
{
this(CassandraRelevantProperties.SNAPSHOT_CLEANUP_INITIAL_DELAY_SECONDS.getInt(),
CassandraRelevantProperties.SNAPSHOT_CLEANUP_PERIOD_SECONDS.getInt());
}
@VisibleForTesting
protected SnapshotManager(long initialDelaySeconds, long
cleanupPeriodSeconds)
+ {
+ this(initialDelaySeconds, cleanupPeriodSeconds,
DatabaseDescriptor.getAllDataFileLocations());
+ }
+
+ @VisibleForTesting
+ protected SnapshotManager(long initialDelaySeconds, long
cleanupPeriodSeconds, String[] dataDirs)
{
this.initialDelaySeconds = initialDelaySeconds;
this.cleanupPeriodSeconds = cleanupPeriodSeconds;
- snapshotLoader = new
SnapshotLoader(DatabaseDescriptor.getAllDataFileLocations());
+ snapshotLoader = new SnapshotLoader(dataDirs);
+ snapshotRateLimiter = DatabaseDescriptor.getSnapshotRateLimiter();
}
- public Collection<TableSnapshot> getExpiringSnapshots()
+ public void registerMBean()
{
- return expiringSnapshots;
+ logger.debug("Registering SnapshotManagerMBean");
+ MBeanWrapper.instance.registerMBean(this, MBEAN_NAME);
}
- public synchronized void start()
+ public void unregisterMBean()
+ {
+ MBeanWrapper.instance.unregisterMBean(MBEAN_NAME);
+ }
+
+ public static void shutdownAndWait(long timeout, TimeUnit unit) throws
InterruptedException, TimeoutException
+ {
+ ExecutorUtils.shutdownNowAndWait(timeout, unit, executor);
+ }
+
+ public synchronized void start(boolean runPeriodicSnapshotCleaner)
{
addSnapshots(loadSnapshots());
- resumeSnapshotCleanup();
+ if (runPeriodicSnapshotCleaner)
+ resumeSnapshotCleanup();
+ }
+
+ public synchronized void start()
+ {
+ start(false);
}
- public synchronized void stop() throws InterruptedException,
TimeoutException
+ @Override
+ public synchronized void close()
{
+ pauseSnapshotCleanup();
expiringSnapshots.clear();
+ liveSnapshots.clear();
+ }
+
+ public synchronized void close(boolean shutdownExecutor) throws Exception
+ {
+ close();
+ if (shutdownExecutor)
+ shutdownAndWait(1, TimeUnit.MINUTES);
+ }
+
+ public synchronized Set<TableSnapshot> loadSnapshots()
+ {
+ return snapshotLoader.loadSnapshots();
+ }
+
+ public synchronized void restart()
+ {
+ restart(true);
+ }
+
+ public synchronized void restart(boolean runPeriodicSnapshotCleaner)
+ {
+ logger.debug("Restarting SnapshotManager");
+ close();
+ start(runPeriodicSnapshotCleaner);
+ logger.debug("SnapshotManager restarted");
+ }
+
+ synchronized void addSnapshot(TableSnapshot snapshot)
+ {
+ logger.debug("Adding snapshot {}", snapshot);
+
+ if (snapshot.isExpiring())
+ expiringSnapshots.add(snapshot);
+ else
+ liveSnapshots.add(snapshot);
+ }
+
+ synchronized void addSnapshots(Collection<TableSnapshot> snapshots)
+ {
+ snapshots.forEach(this::addSnapshot);
+ }
+
+ public synchronized void resumeSnapshotCleanup()
+ {
+ if (cleanupTaskFuture == null)
+ {
+ logger.info("Scheduling expired snapshots cleanup with
initialDelaySeconds={} and cleanupPeriodSeconds={}",
+ initialDelaySeconds, cleanupPeriodSeconds);
+
+ cleanupTaskFuture =
executor.scheduleWithFixedDelay(this::clearExpiredSnapshots,
+
initialDelaySeconds,
+
cleanupPeriodSeconds,
+ SECONDS);
+ }
+ }
+
+ synchronized void pauseSnapshotCleanup()
+ {
if (cleanupTaskFuture != null)
{
cleanupTaskFuture.cancel(false);
cleanupTaskFuture = null;
}
}
- public synchronized void addSnapshot(TableSnapshot snapshot)
+ /**
+ * Deletes snapshot and removes it from manager.
+ *
+ * @param snapshot snapshot to clear
+ */
+ synchronized void clearSnapshot(TableSnapshot snapshot)
{
- // We currently only care about expiring snapshots
- if (snapshot.isExpiring())
+ clearSnapshot(snapshot, true);
+ }
+
+ synchronized void clearSnapshot(TableSnapshot snapshot, boolean deleteData)
+ {
+ logger.debug("Removing snapshot {}{}", snapshot, deleteData ? ",
deleting data" : "");
+
+ if (deleteData)
{
- logger.debug("Adding expiring snapshot {}", snapshot);
- expiringSnapshots.add(snapshot);
+ for (File snapshotDir : snapshot.getDirectories())
+ {
+ try
+ {
+ removeSnapshotDirectory(snapshotDir);
+ }
+ catch (Exception ex)
+ {
+ logger.warn("Unable to remove snapshot directory {}",
snapshotDir, ex);
+ }
+ }
}
+
+ if (snapshot.isExpiring())
+ expiringSnapshots.remove(snapshot);
+ else
+ liveSnapshots.remove(snapshot);
}
- public synchronized Set<TableSnapshot> loadSnapshots(String keyspace)
+ /**
+ * Returns list of snapshots of given keyspace
+ *
+ * @param keyspace keyspace of a snapshot
+ * @return list of snapshots of given keyspace.
+ */
+ public List<TableSnapshot> getSnapshots(String keyspace)
{
- return snapshotLoader.loadSnapshots(keyspace);
+ return getSnapshots(snapshot ->
snapshot.getKeyspaceName().equals(keyspace));
}
- public synchronized Set<TableSnapshot> loadSnapshots()
+ /**
+ * Returns list of snapshots from given keyspace and table.
+ *
+ * @param keyspace keyspace of a snapshot
+ * @param table table of a snapshot
+ * @return list of snapshots from given keyspace and table
+ */
+ public List<TableSnapshot> getSnapshots(String keyspace, String table)
{
- return snapshotLoader.loadSnapshots();
+ return getSnapshots(snapshot ->
snapshot.getKeyspaceName().equals(keyspace) &&
+ snapshot.getTableName().equals(table));
}
- @VisibleForTesting
- protected synchronized void addSnapshots(Collection<TableSnapshot>
snapshots)
+ /**
+ * Returns a snapshot or empty optional based on the given parameters.
+ *
+ * @param keyspace keyspace of a snapshot
+ * @param table table of a snapshot
+ * @param tag name of a snapshot
+ * @return empty optional if there is not such snapshot, non-empty
otherwise
+ */
+ public synchronized Optional<TableSnapshot> getSnapshot(String keyspace,
String table, String tag)
{
- logger.debug("Adding snapshots: {}.", Joiner.on(",
").join(snapshots.stream().map(TableSnapshot::getId).collect(toList())));
- snapshots.forEach(this::addSnapshot);
+ // we do not use the predicate here because we want to stop the loop
as soon as
+ // we find the snapshot we are looking for, looping until the end is
not necessary
+ for (TableSnapshot snapshot : Iterables.concat(liveSnapshots,
expiringSnapshots))
+ {
+ if (snapshot.getKeyspaceName().equals(keyspace) &&
+ snapshot.getTableName().equals(table) &&
+ snapshot.getTag().equals(tag) || (tag != null &&
tag.isEmpty()))
+ {
+ return Optional.of(snapshot);
+ }
+ }
+
+ return Optional.empty();
}
- // TODO: Support pausing snapshot cleanup
- @VisibleForTesting
- synchronized void resumeSnapshotCleanup()
+ /**
+ * Return snapshots based on given parameters.
+ *
+ * @param skipExpiring if expiring snapshots should be skipped
+ * @param includeEphemeral if ephemeral snapshots should be included
+ * @return snapshots based on given parameters
+ */
+ public List<TableSnapshot> getSnapshots(boolean skipExpiring, boolean
includeEphemeral)
{
- if (cleanupTaskFuture == null)
+ return getSnapshots(s -> (!skipExpiring || !s.isExpiring()) &&
(includeEphemeral || !s.isEphemeral()));
+ }
+
+ /**
+ * @return all ephemeral snapshots in a node
+ */
+ public List<TableSnapshot> getEphemeralSnapshots()
+ {
+ return getSnapshots(TableSnapshot::isEphemeral);
+ }
+
+ /**
+ * Returns all snapshots passing the given predicate.
+ *
+ * @param predicate predicate to filter all snapshots of
+ * @return list of snapshots passing the predicate
+ */
+ public synchronized List<TableSnapshot>
getSnapshots(Predicate<TableSnapshot> predicate)
+ {
+ List<TableSnapshot> notExistingAnymore = new ArrayList<>();
+ List<TableSnapshot> snapshots = new ArrayList<>();
+ for (TableSnapshot snapshot : Iterables.concat(liveSnapshots,
expiringSnapshots))
{
- logger.info("Scheduling expired snapshot cleanup with
initialDelaySeconds={} and cleanupPeriodSeconds={}",
- initialDelaySeconds, cleanupPeriodSeconds);
- cleanupTaskFuture =
executor.scheduleWithFixedDelay(this::clearExpiredSnapshots,
initialDelaySeconds,
-
cleanupPeriodSeconds, TimeUnit.SECONDS);
+ if (predicate.test(snapshot))
+ {
+ if (!snapshot.hasManifest())
+ notExistingAnymore.add(snapshot);
+ else
+ snapshots.add(snapshot);
+ }
}
+
+ for (TableSnapshot tableSnapshot : notExistingAnymore)
+ clearSnapshot(tableSnapshot, false);
+
+ return snapshots;
}
- @VisibleForTesting
- protected synchronized void clearExpiredSnapshots()
+ public Collection<TableSnapshot> getExpiringSnapshots()
+ {
+ return expiringSnapshots;
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspaces.
+ * <p>
+ * If tag is not present / is empty, all snapshots are considered to be
cleared.
+ * If keyspaces are empty, all snapshots of given tag and older than
maxCreatedAt are removed.
+ * <p>
+ * Ephemeral snapshots are not included.
+ *
+ * @param tag optional tag of snapshot to clear
+ * @param keyspaces keyspaces to remove snapshots for
+ * @param maxCreatedAt clear all such snapshots which were created before
this timestamp
+ */
+ public void clearSnapshots(String tag, Set<String> keyspaces, long
maxCreatedAt)
+ {
+ clearSnapshots(tag, keyspaces, maxCreatedAt, false);
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspace.
+ * <p>
+ *
+ * @param tag snapshot name
+ * @param keyspace keyspace to clear all snapshots of a given tag of
+ */
+ public void clearSnapshots(String tag, String keyspace)
+ {
+ clearSnapshots(tag, Set.of(keyspace),
Clock.Global.currentTimeMillis(), false);
+ }
+
+ /**
+ * Removes a snapshot.
+ * <p>
+ *
+ * @param keyspace keyspace of a snapshot to remove
+ * @param table table of a snapshot to remove
+ * @param tag name of a snapshot to remove.
+ */
+ public void clearSnapshot(String keyspace, String table, String tag)
+ {
+ getSnapshot(keyspace, table, tag).ifPresent(this::clearSnapshot);
+ }
+
+ /**
+ * Clears all ephemeral snapshots in a node.
+ */
+ public void clearEphemeralSnapshots()
{
- TableSnapshot expiredSnapshot;
- while ((expiredSnapshot = expiringSnapshots.peek()) != null)
+ getEphemeralSnapshots().forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Clears all expired snapshots in a node.
+ */
+ public synchronized void clearExpiredSnapshots()
+ {
+ Instant now = FBUtilities.now();
+ getSnapshots(s -> s.isExpired(now)).forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Clear snapshots of given tag from given keyspaces.
+ * <p>
+ * If tag is not present / is empty, all snapshots are considered to be
cleared.
+ * If keyspaces are empty, all snapshots of given tag and older than
maxCreatedAt are removed.
+ *
+ * @param tag optional tag of snapshot to clear
+ * @param keyspaces keyspaces to remove snapshots for
+ * @param maxCreatedAt clear all such snapshots which were created
before this timestamp
+ * @param includeEphemeral include ephemeral snaphots for removal or not
+ */
+ synchronized void clearSnapshots(String tag, Set<String> keyspaces,
+ long maxCreatedAt,
+ boolean includeEphemeral)
+ {
+ Predicate<TableSnapshot> predicate = shouldClearSnapshot(tag,
keyspaces, maxCreatedAt, includeEphemeral);
+ getSnapshots(predicate).forEach(this::clearSnapshot);
+ }
+
+ /**
+ * Returns a predicate based on which a snapshot will be included for
deletion or not.
+ *
+ * @param tag name of snapshot to remove
+ * @param keyspaces keyspaces this snapshot belongs to
+ * @param olderThanTimestamp clear the snapshot if it is older than given
timestamp
+ * @param includeEphemeral whether to include ephemeral snapshots as well
+ * @return predicate which filters snapshots on given parameters
+ */
+ static Predicate<TableSnapshot> shouldClearSnapshot(String tag,
+ Set<String> keyspaces,
+ long
olderThanTimestamp,
+ boolean
includeEphemeral)
+ {
+ return ts ->
{
- if (!expiredSnapshot.isExpired(now()))
- break; // the earliest expiring snapshot is not expired yet,
so there is no more expired snapshots to remove
+ // When no tag is supplied, all snapshots must be cleared
+ boolean clearAll = tag == null || tag.isEmpty();
+ if (!clearAll && ts.isEphemeral() && !includeEphemeral)
+ logger.info("Skipping deletion of ephemeral snapshot '{}' in
keyspace {}. " +
+ "Ephemeral snapshots are not removable by a user.",
+ tag, ts.getKeyspaceName());
+ boolean passedEphemeralTest = !ts.isEphemeral() ||
(ts.isEphemeral() && includeEphemeral);
+ boolean shouldClearTag = clearAll || ts.getTag().equals(tag);
+ boolean byTimestamp = true;
- logger.debug("Removing expired snapshot {}.", expiredSnapshot);
- clearSnapshot(expiredSnapshot);
- }
+ if (olderThanTimestamp > 0L)
+ {
+ Instant createdAt = ts.getCreatedAt();
+ if (createdAt != null)
+ byTimestamp =
createdAt.isBefore(Instant.ofEpochMilli(olderThanTimestamp));
+ }
+
+ boolean byKeyspace = (keyspaces.isEmpty() ||
keyspaces.contains(ts.getKeyspaceName()));
+
+ return passedEphemeralTest && shouldClearTag && byTimestamp &&
byKeyspace;
+ };
}
/**
- * Deletes snapshot and remove it from manager
+ * Takes a snapshot by creating hardlinks into snapshot directories. This
method also
+ * creates manifests and schema files and such snapshot will be added
among tracked ones in this manager.
+ *
+ * @param cfs column family to create a snapshot for
+ * @param tag name of snapshot
+ * @param ephemeral true if the snapshot is ephemeral, false otherwise
+ * @param ttl time after the created snapshot will be removed
+ * @param creationTime time the snapshot was created
+ * @param rateLimiter limiter for hard-links creation, if null, limiter
from DatabaseDescriptor will be used
+ * @return logical representation of a snapshot
*/
- public synchronized void clearSnapshot(TableSnapshot snapshot)
+ public TableSnapshot createSnapshot(ColumnFamilyStore cfs,
+ String tag,
+
com.google.common.base.Predicate<SSTableReader> predicate,
+ boolean ephemeral,
+ DurationSpec.IntSecondsBound ttl,
+ Instant creationTime,
+ RateLimiter rateLimiter)
{
- for (File snapshotDir : snapshot.getDirectories())
-
Directories.removeSnapshotDirectory(DatabaseDescriptor.getSnapshotRateLimiter(),
snapshotDir);
+ if (ephemeral && ttl != null)
+ throw new IllegalStateException(format("can not take ephemeral
snapshot (%s) while ttl is specified too", tag));
+
+ RateLimiter limiter = rateLimiter;
+ if (limiter == null)
+ limiter = SnapshotManager.instance.snapshotRateLimiter;
+
+ Set<SSTableReader> sstables = new LinkedHashSet<>();
+ for (ColumnFamilyStore aCfs : cfs.concatWithIndexes())
+ {
+ try (ColumnFamilyStore.RefViewFragment currentView =
aCfs.selectAndReference(View.select(SSTableSet.CANONICAL, (x) -> predicate ==
null || predicate.apply(x))))
+ {
+ for (SSTableReader ssTable : currentView.sstables)
+ {
+ File snapshotDirectory =
Directories.getSnapshotDirectory(ssTable.descriptor, tag);
+ ssTable.createLinks(snapshotDirectory.path(), limiter); //
hard links
+ if (logger.isTraceEnabled())
+ logger.trace("Snapshot for {} keyspace data file {}
created in {}", cfs.keyspace, ssTable.getFilename(), snapshotDirectory);
+ sstables.add(ssTable);
+ }
+ }
+ }
+
+ List<String> dataComponents = new ArrayList<>();
+ for (SSTableReader sstable : sstables)
+
dataComponents.add(sstable.descriptor.relativeFilenameFor(SSTableFormat.Components.DATA));
+
+ SnapshotManifest manifest = new SnapshotManifest(dataComponents, ttl,
creationTime, ephemeral);
+
+ Set<File> snapshotDirs = cfs.getDirectories().getSnapshotDirs(tag);
- expiringSnapshots.remove(snapshot);
+ for (File snapshotDir : snapshotDirs)
+ {
+ writeSnapshotManifest(manifest,
Directories.getSnapshotManifestFile(snapshotDir));
+
+ if (!SchemaConstants.isLocalSystemKeyspace(cfs.metadata.keyspace)
+ &&
!SchemaConstants.isReplicatedSystemKeyspace(cfs.metadata.keyspace))
+ {
+
writeSnapshotSchema(Directories.getSnapshotSchemaFile(snapshotDir), cfs);
+ }
+ }
+
+ TableSnapshot snapshot = new TableSnapshot(cfs.metadata.keyspace,
+ cfs.metadata.name,
+ cfs.metadata.id.asUUID(),
+ tag,
+ creationTime,
+
SnapshotManifest.computeExpiration(ttl, creationTime),
+ snapshotDirs,
+ ephemeral);
+
+ addSnapshot(snapshot);
+ return snapshot;
}
- @VisibleForTesting
- public static void shutdownAndWait(long timeout, TimeUnit unit) throws
InterruptedException, TimeoutException
+ // MBean methods
+
+ @Override
+ public void takeSnapshot(String tag, Map<String, String> options,
String... entities) throws IOException
{
- ExecutorUtils.shutdownNowAndWait(timeout, unit, executor);
+ if (StorageService.instance.operationMode() ==
StorageService.Mode.JOINING)
+ throw new IOException("Cannot snapshot until bootstrap completes");
+
+ if (tag == null || tag.isEmpty())
+ throw new IOException("You must supply a snapshot name.");
+
+ DurationSpec.IntSecondsBound ttl = options.containsKey("ttl") ? new
DurationSpec.IntSecondsBound(options.get("ttl")) : null;
+ if (ttl != null)
+ {
+ int minAllowedTtlSecs =
CassandraRelevantProperties.SNAPSHOT_MIN_ALLOWED_TTL_SECONDS.getInt();
+ if (ttl.toSeconds() < minAllowedTtlSecs)
+ throw new IllegalArgumentException(format("ttl for snapshot
must be at least %d seconds", minAllowedTtlSecs));
+ }
+
+ boolean skipFlush =
Boolean.parseBoolean(options.getOrDefault("skipFlush", "false"));
+
+ Map<Keyspace, Set<ColumnFamilyStore>> entitiesForSnapshot =
parseEntitiesForSnapshot(entities);
+
+ for (Map.Entry<Keyspace, Set<ColumnFamilyStore>> entry :
entitiesForSnapshot.entrySet())
+ {
+ for (ColumnFamilyStore table : entry.getValue())
+ {
+ String keyspaceName = table.getKeyspaceName();
+ String tableName = table.getTableName();
+ if (getSnapshot(table.getKeyspaceName(), table.getTableName(),
tag).isPresent())
+ throw new IOException(format("Snapshot %s for %s.%s
already exists.", tag, keyspaceName, tableName));
+ }
+ }
+
+ Instant creationTime = now();
+
+ for (Map.Entry<Keyspace, Set<ColumnFamilyStore>> entry :
entitiesForSnapshot.entrySet())
+ {
+ Keyspace keyspace = entry.getKey();
+ for (ColumnFamilyStore table : entry.getValue())
+ keyspace.snapshot(tag, table.getTableName(), skipFlush, ttl,
snapshotRateLimiter, creationTime);
Review Comment:
We already have all the `ColumnFamilyStore` objects we want to snapshot
here. Why can't we bypass `Keyspace` altogether and just call
`ColumnFamilyStore.snapshot` for each snapshot in the `entitiesForSnapshot` map
?
I think this would allow removing or simplifying most snapshot logic from
[Keyspace](https://github.com/apache/cassandra/blob/fe025c7f79e76d99e0db347518a7872fd4a114bc/src/java/org/apache/cassandra/db/Keyspace.java#L239)
and ColumnFamilyStore which is ultimately calling
[SnapshotManager.instance.createSnapshot](https://github.com/instaclustr/cassandra/blob/CASSANDRA-18111/src/java/org/apache/cassandra/db/ColumnFamilyStore.java#L2138).
Lmk if this makes sense.
--
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]
