You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2022/06/19 20:38:45 UTC
[GitHub] [cassandra] pauloricardomg commented on a diff in pull request #1184: CASSANDRA-16911 - remove ephemeral marker file for snapshot
pauloricardomg commented on code in PR #1184:
URL: https://github.com/apache/cassandra/pull/1184#discussion_r901152796
##########
src/java/org/apache/cassandra/db/ColumnFamilyStore.java:
##########
@@ -817,14 +829,6 @@ public static void scrubDataDirectories(TableMetadata metadata) throws StartupE
if (!file.tryDelete())
logger.warn("could not delete {}", file.absolutePath());
}
-
Review Comment:
why is this being removed?
##########
src/java/org/apache/cassandra/db/Directories.java:
##########
@@ -977,24 +966,30 @@ public Map<String, TableSnapshot> listSnapshots()
String tag = entry.getKey();
Set<File> snapshotDirs = entry.getValue();
SnapshotManifest manifest = maybeLoadManifest(metadata.keyspace, metadata.name, tag, snapshotDirs);
- snapshots.put(tag, buildSnapshot(tag, manifest, snapshotDirs));
+ boolean ephemeral = manifest == null ? isEphemeral(snapshotDirs) : manifest.ephemeral;
+ snapshots.put(tag, buildSnapshot(tag, manifest, snapshotDirs, ephemeral));
}
return snapshots;
}
- protected TableSnapshot buildSnapshot(String tag, SnapshotManifest manifest, Set<File> snapshotDirs) {
+ protected TableSnapshot buildSnapshot(String tag, SnapshotManifest manifest, Set<File> snapshotDirs, boolean ephemeral) {
Instant createdAt = manifest == null ? null : manifest.createdAt;
Instant expiresAt = manifest == null ? null : manifest.expiresAt;
return new TableSnapshot(metadata.keyspace, metadata.name, metadata.id.asUUID(), tag, createdAt, expiresAt,
- snapshotDirs);
+ snapshotDirs, ephemeral);
+ }
+
+ protected static boolean isEphemeral(Set<File> snapshotDirs)
Review Comment:
Can you add a short comment about why this method is needed - should we remove it at some point and just use the manifest to decide if a snapshot is ephemeral?
##########
src/java/org/apache/cassandra/db/ColumnFamilyStore.java:
##########
@@ -1698,9 +1702,9 @@ public CompactionManager.AllSSTableOpStatus verify(Verifier.Options options) thr
/**
* Rewrites all SSTables according to specified parameters
*
- * @param skipIfCurrentVersion - if {@link true}, will rewrite only SSTables that have version older than the current one ({@link BigFormat#latestVersion})
+ * @param skipIfCurrentVersion - if {@link true}, will rewrite only SSTables that have version older than the current one ({@link org.apache.cassandra.io.sstable.format.big.BigFormat#latestVersion})
* @param skipIfNewerThanTimestamp - max timestamp (local creation time) for SSTable; SSTables created _after_ this timestamp will be excluded from compaction
- * @param skipIfCompressionMatches - if {@link true}, will rewrite only SSTables whose compression parameters are different from {@link CFMetaData#compressionParams()}
+ * @param skipIfCompressionMatches - if {@link true}, will rewrite only SSTables whose compression parameters are different from {@link TableMetadata#params#getCompressionParameters()} ()}
Review Comment:
👍
##########
src/java/org/apache/cassandra/db/Directories.java:
##########
@@ -1018,42 +1013,6 @@ protected static SnapshotManifest maybeLoadManifest(String keyspace, String tabl
return null;
}
- public List<String> listEphemeralSnapshots()
- {
- final List<String> ephemeralSnapshots = new LinkedList<>();
- for (File snapshot : listAllSnapshots())
- {
- if (getEphemeralSnapshotMarkerFile(snapshot).exists())
- ephemeralSnapshots.add(snapshot.name());
- }
- return ephemeralSnapshots;
- }
-
- private List<File> listAllSnapshots()
Review Comment:
why is this removed?
##########
src/java/org/apache/cassandra/db/ColumnFamilyStore.java:
##########
@@ -1698,9 +1702,9 @@ public CompactionManager.AllSSTableOpStatus verify(Verifier.Options options) thr
/**
* Rewrites all SSTables according to specified parameters
*
- * @param skipIfCurrentVersion - if {@link true}, will rewrite only SSTables that have version older than the current one ({@link BigFormat#latestVersion})
+ * @param skipIfCurrentVersion - if {@link true}, will rewrite only SSTables that have version older than the current one ({@link org.apache.cassandra.io.sstable.format.big.BigFormat#latestVersion})
Review Comment:
👍
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org