This is an automated email from the ASF dual-hosted git repository.

smiklosovic pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git

commit 8d54835d5b84f8afe1e97be65f8cc1897b0a9c5f
Merge: d7352209b2 cfe9641fbe
Author: Stefan Miklosovic <smikloso...@apache.org>
AuthorDate: Fri Feb 17 15:44:17 2023 +0100

    Merge branch 'cassandra-4.1' into trunk

 CHANGES.txt                                        |   1 +
 src/java/org/apache/cassandra/db/Directories.java  |   6 +-
 .../org/apache/cassandra/io/util/FileUtils.java    |  12 ++
 .../apache/cassandra/service/StorageService.java   |  20 +--
 .../cassandra/service/snapshot/SnapshotLoader.java | 152 +++++++++++----------
 .../service/snapshot/SnapshotManager.java          |  53 +++----
 .../cassandra/distributed/impl/Instance.java       |   1 +
 .../cassandra/distributed/test/SnapshotsTest.java  |  55 ++++----
 .../service/snapshot/SnapshotLoaderTest.java       |   2 +-
 .../service/snapshot/SnapshotManagerTest.java      |  82 +++++++++--
 10 files changed, 242 insertions(+), 142 deletions(-)

diff --cc CHANGES.txt
index 1b90fee599,decc10dd72..76f6d41c83
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@@ -1,111 -1,5 +1,112 @@@
 -4.1.1
 +4.2
 + * Prepare for JDK17 experimental support (CASSANDRA-18179)
 + * Remove Scripted UDFs internals; hooks to be added later in CASSANDRA-17281 
(CASSANDRA-18252)
 + * Update JNA to 5.13.0 (CASSANDRA-18050)
 + * Make virtual tables decide if they implicitly enable ALLOW FILTERING 
(CASSANDRA-18238)
 + * Add row, tombstone, and sstable count to nodetool profileload 
(CASSANDRA-18022)
 + * Coordinator level metrics for read response and mutation row and column 
counts (CASSANDRA-18155)
 + * Add CQL functions for dynamic data masking (CASSANDRA-17941)
 + * Print friendly error when nodetool attempts to connect to uninitialized 
server (CASSANDRA-11537)
 + * Use G1GC by default, and update default G1GC settings (CASSANDRA-18027)
 + * SimpleSeedProvider can resolve multiple IP addresses per DNS record 
(CASSANDRA-14361)
 + * Remove mocking in InternalNodeProbe spying on StorageServiceMBean 
(CASSANDRA-18152)
 + * Add compaction_properties column to system.compaction_history table and 
nodetool compactionhistory command (CASSANDRA-18061)
 + * Remove ProtocolVersion entirely from the CollectionSerializer ecosystem 
(CASSANDRA-18114)
 + * Fix serialization error in new getsstables --show-levels option 
(CASSANDRA-18140)
 + * Use checked casts when reading vints as ints (CASSANDRA-18099)
 + * Add Mutation Serialization Caching (CASSANDRA-17998)
 + * Only reload compaction strategies if disk boundaries change 
(CASSANDRA-17874)
 + * CEP-10: Simulator Java11 Support (CASSANDRA-17178)
 + * Set the major compaction type correctly for compactionstats 
(CASSANDRA-18055)
 + * Print exception message without stacktrace when nodetool commands fail on 
probe.getOwnershipWithPort() (CASSANDRA-18079)
 + * Add option to print level in nodetool getsstables output (CASSANDRA-18023)
 + * Implement a guardrail for not having zero default_time_to_live on tables 
with TWCS (CASSANDRA-18042)
 + * Add CQL scalar functions for collection aggregation (CASSANDRA-18060)
 + * Make cassandra.replayList property for CommitLogReplayer possible to react 
on keyspaces only (CASSANDRA-18044)
 + * Add Mathematical functions (CASSANDRA-17221)
 + * Make incremental backup configurable per table (CASSANDRA-15402)
 + * Change shebangs of Python scripts to resolve Python 3 from env command 
(CASSANDRA-17832)
 + * Add reasons to guardrail messages and consider guardrails in the error 
message for needed ALLOW FILTERING (CASSANDRA-17967)
 + * Add support for CQL functions on collections, tuples and UDTs 
(CASSANDRA-17811)
 + * Add flag to exclude nodes from local DC when running nodetool rebuild 
(CASSANDRA-17870)
 + * Adding endpoint verification option to client_encryption_options 
(CASSANDRA-18034)
 + * Replace 'wcwidth.py' with pypi module (CASSANDRA-17287)
 + * Add nodetool forcecompact to remove tombstoned or ttl'd data ignoring GC 
grace for given table and partition keys (CASSANDRA-17711)
 + * Offer IF (NOT) EXISTS in cqlsh completion for CREATE TYPE, DROP TYPE, 
CREATE ROLE and DROP ROLE (CASSANDRA-16640)
 + * Nodetool bootstrap resume will now return an error if the operation fails 
(CASSANDRA-16491)
 + * Disable resumable bootstrap by default (CASSANDRA-17679)
 + * Include Git SHA in --verbose flag for nodetool version (CASSANDRA-17753)
 + * Update Byteman to 4.0.20 and Jacoco to 0.8.8 (CASSANDRA-16413)
 + * Add memtable option among possible tab completions for a table 
(CASSANDRA-17982)
 + * Adds a trie-based memtable implementation (CASSANDRA-17240)
 + * Further improves precision of memtable heap tracking (CASSANDRA-17240)
 + * Fix formatting of metrics documentation (CASSANDRA-17961)
 + * Keep sstable level when streaming for decommission and move 
(CASSANDRA-17969)
 + * Add Unavailables metric for CASWrite in the docs (CASSANDRA-16357)
 + * Make Cassandra logs able to be viewed in the virtual table 
system_views.system_logs (CASSANDRA-17946)
 + * IllegalArgumentException in Gossiper#order due to concurrent mutations to 
elements being applied (CASSANDRA-17908)
 + * Include estimated active compaction remaining write size when starting a 
new compaction (CASSANDRA-17931)
 + * Mixed mode support for internode authentication during TLS upgrades 
(CASSANDRA-17923)
 + * Revert Mockito downgrade from CASSANDRA-17750 (CASSANDRA-17496)
 + * Add --older-than and --older-than-timestamp options for nodetool 
clearsnapshots (CASSANDRA-16860)
 + * Fix "open RT bound as its last item" exception (CASSANDRA-17810)
 + * Fix leak of non-standard Java types in JMX MBeans 
`org.apache.cassandra.db:type=StorageService`
 +   and `org.apache.cassandra.db:type=RepairService` as clients using JMX 
cannot handle them. More details in NEWS.txt (CASSANDRA-17668)
 + * Deprecate Throwables.propagate usage (CASSANDRA-14218)
 + * Allow disabling hotness persistence for high sstable counts 
(CASSANDRA-17868)
 + * Prevent NullPointerException when changing neverPurgeTombstones from true 
to false (CASSANDRA-17897)
 + * Add metrics around storage usage and compression (CASSANDRA-17898)
 + * Remove usage of deprecated javax certificate classes (CASSANDRA-17867)
 + * Make sure preview repairs don't optimise streams unless configured to 
(CASSANDRA-17865)
 + * Optionally avoid hint transfer during decommission (CASSANDRA-17808)
 + * Make disabling auto snapshot on selected tables possible (CASSANDRA-10383)
 + * Introduce compaction priorities to prevent upgrade compaction inability to 
finish (CASSANDRA-17851)
 + * Prevent a user from manually removing ephemeral snapshots (CASSANDRA-17757)
 + * Remove dependency on Maven Ant Tasks (CASSANDRA-17750)
 + * Update ASM(9.1 to 9.3), Mockito(1.10.10 to 1.12.13) and ByteBuddy(3.2.4 to 
4.7.0) (CASSANDRA-17835)
 + * Add the ability for operators to loosen the definition of "empty" for edge 
cases (CASSANDRA-17842)
 + * Fix potential out of range exception on column index downsampling 
(CASSANDRA-17839)
 + * Introduce target directory to vtable output for sstable_tasks and for 
compactionstats (CASSANDRA-13010)
 + * Read/Write/Truncate throw RequestFailure in a race condition with callback 
timeouts, should return Timeout instead (CASSANDRA-17828)
 + * Add ability to log load profiles at fixed intervals (CASSANDRA-17821)
 + * Protect against Gossip backing up due to a quarantined endpoint without 
version information (CASSANDRA-17830)
 + * NPE in org.apache.cassandra.cql3.Attributes.getTimeToLive (CASSANDRA-17822)
 + * Add guardrail for column size (CASSANDRA-17151)
 + * When doing a host replacement, we need to check that the node is a live 
node before failing with "Cannot replace a live node..." (CASSANDRA-17805)
 + * Add support to generate a One-Shot heap dump on unhandled exceptions 
(CASSANDRA-17795)
 + * Rate-limit new client connection auth setup to avoid overwhelming bcrypt 
(CASSANDRA-17812)
 + * DataOutputBuffer#scratchBuffer can use off-heap or on-heap memory as a 
means to control memory allocations (CASSANDRA-16471)
 + * Add ability to read the TTLs and write times of the elements of a 
collection and/or UDT (CASSANDRA-8877)
 + * Removed Python < 2.7 support from formatting.py (CASSANDRA-17694)
 + * Cleanup pylint issues with pylexotron.py (CASSANDRA-17779)
 + * NPE bug in streaming checking if SSTable is being repaired 
(CASSANDRA-17801)
 + * Users of NativeLibrary should handle lack of JNA appropriately when 
running in client mode (CASSANDRA-17794)
 + * Warn on unknown directories found in system keyspace directory rather than 
kill node during startup checks (CASSANDRA-17777)
 + * Log duplicate rows sharing a partition key found in verify and scrub 
(CASSANDRA-17789)
 + * Add separate thread pool for Secondary Index building so it doesn't block 
compactions (CASSANDRA-17781)
 + * Added JMX call to getSSTableCountPerTWCSBucket for TWCS (CASSANDRA-17774)
 + * When doing a host replacement, -Dcassandra.broadcast_interval_ms is used 
to know when to check the ring but checks that the ring wasn't changed in 
-Dcassandra.ring_delay_ms, changes to ring delay should not depend on when we 
publish load stats (CASSANDRA-17776)
 + * When bootstrap fails, CassandraRoleManager may attempt to do read queries 
that fail with "Cannot read from a bootstrapping node", and increments 
unavailables counters (CASSANDRA-17754)
 + * Add guardrail to disallow DROP KEYSPACE commands (CASSANDRA-17767)
 + * Remove ephemeral snapshot marker file and introduce a flag to 
SnapshotManifest (CASSANDRA-16911)
 + * Add a virtual table that exposes currently running queries 
(CASSANDRA-15241)
 + * Allow sstableloader to specify table without relying on path 
(CASSANDRA-16584)
 + * Fix 
TestGossipingPropertyFileSnitch.test_prefer_local_reconnect_on_listen_address 
(CASSANDRA-17700)
 + * Add ByteComparable API (CASSANDRA-6936)
 + * Add guardrail for maximum replication factor (CASSANDRA-17500)
 + * Increment CQLSH to version 6.2.0 for release 4.2 (CASSANDRA-17646)
 + * Adding support to perform certificate based internode authentication 
(CASSANDRA-17661)
 + * Option to disable CDC writes of repaired data (CASSANDRA-17666)
 + * When a node is bootstrapping it gets the whole gossip state but applies in 
random order causing some cases where StorageService will fail causing an 
instance to not show up in TokenMetadata (CASSANDRA-17676)
 + * Add CQLSH command SHOW REPLICAS (CASSANDRA-17577)
 + * Add guardrail to allow disabling of SimpleStrategy (CASSANDRA-17647)
 + * Change default directory permission to 750 in packaging (CASSANDRA-17470)
 + * Adding support for TLS client authentication for internode communication 
(CASSANDRA-17513)
 + * Add new CQL function maxWritetime (CASSANDRA-17425)
 + * Add guardrail for ALTER TABLE ADD / DROP / REMOVE column operations 
(CASSANDRA-17495)
 + * Rename DisableFlag class to EnableFlag on guardrails (CASSANDRA-17544)
 +Merged from 4.1:
+  * Fix possible NoSuchFileException when removing a snapshot (CASSANDRA-18211)
   * PaxosPrepare may add instances to the Electorate that are not in gossip 
(CASSANDRA-18194)
   * Fix PAXOS2_COMMIT_AND_PREPARE_RSP serialisation AssertionError 
(CASSANDRA-18164)
   * Streaming progress virtual table lock contention can trigger 
TCP_USER_TIMEOUT and fail streaming (CASSANDRA-18110)
diff --cc src/java/org/apache/cassandra/service/StorageService.java
index 10f6e672de,dd06ac41b7..74a15da07b
--- a/src/java/org/apache/cassandra/service/StorageService.java
+++ b/src/java/org/apache/cassandra/service/StorageService.java
@@@ -70,11 -70,8 +70,10 @@@ import org.apache.cassandra.fql.FullQue
  import org.apache.cassandra.fql.FullQueryLoggerOptionsCompositeData;
  import org.apache.cassandra.io.util.File;
  import org.apache.cassandra.locator.ReplicaCollection.Builder.Conflict;
 +import org.apache.cassandra.metrics.Sampler;
 +import org.apache.cassandra.metrics.SamplingManager;
  import org.apache.cassandra.schema.Keyspaces;
  import org.apache.cassandra.service.disk.usage.DiskUsageBroadcaster;
- import org.apache.cassandra.service.snapshot.SnapshotLoader;
  import org.apache.cassandra.utils.concurrent.Future;
  import org.apache.cassandra.schema.TableId;
  import org.apache.cassandra.utils.concurrent.FutureCombiner;
@@@ -4331,32 -4223,13 +4336,31 @@@ public class StorageService extends Not
              logger.debug("Cleared out snapshot directories");
      }
  
 +    /**
 +     * Clear snapshots for a given keyspace.
 +     * @param keyspace keyspace to remove snapshots for
 +     * @param tag the user supplied snapshot name. If empty or null, all the 
snapshots will be cleaned
 +     * @param olderThanTimestamp if a snapshot was created before this 
timestamp, it will be cleared,
 +     *                           if its value is 0, this parameter is 
effectively ignored.
 +     */
 +    private void clearKeyspaceSnapshot(String keyspace, String tag, long 
olderThanTimestamp)
 +    {
-         Set<TableSnapshot> snapshotsToClear = new 
SnapshotLoader().loadSnapshots(keyspace)
-                                                                   .stream()
-                                                                   
.filter(TableSnapshot.shouldClearSnapshot(tag, olderThanTimestamp))
-                                                                   
.collect(Collectors.toSet());
++        Set<TableSnapshot> snapshotsToClear = 
snapshotManager.loadSnapshots(keyspace)
++                                                             .stream()
++                                                             
.filter(TableSnapshot.shouldClearSnapshot(tag, olderThanTimestamp))
++                                                             
.collect(Collectors.toSet());
 +        for (TableSnapshot snapshot : snapshotsToClear)
 +            snapshotManager.clearSnapshot(snapshot);
 +    }
 +
      public Map<String, TabularData> getSnapshotDetails(Map<String, String> 
options)
      {
          boolean skipExpiring = options != null && 
Boolean.parseBoolean(options.getOrDefault("no_ttl", "false"));
 +        boolean includeEphemeral = options != null && 
Boolean.parseBoolean(options.getOrDefault("include_ephemeral", "false"));
  
-         SnapshotLoader loader = new SnapshotLoader();
          Map<String, TabularData> snapshotMap = new HashMap<>();
  
-         for (TableSnapshot snapshot : loader.loadSnapshots())
+         for (TableSnapshot snapshot : snapshotManager.loadSnapshots())
          {
              if (skipExpiring && snapshot.isExpiring())
                  continue;
diff --cc src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java
index b93fb56f31,5f2d37e270..532ea79f1b
--- a/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java
+++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotLoader.java
@@@ -54,11 -55,9 +55,9 @@@ public class SnapshotLoade
  {
      private static final Logger logger = 
LoggerFactory.getLogger(SnapshotLoader.class);
  
 -    static final Pattern SNAPSHOT_DIR_PATTERN = 
Pattern.compile("(?<keyspace>\\w+)/(?<tableName>\\w+)-(?<tableId>[0-9a-f]{32})/snapshots/(?<tag>[\\w-]+)$");
 +    static final Pattern SNAPSHOT_DIR_PATTERN = 
Pattern.compile("(?<keyspace>\\w+)/(?<tableName>\\w+)-(?<tableId>[0-9a-f]{32})/snapshots/(?<tag>.+)$");
-     private static final Pattern UUID_PATTERN = 
Pattern.compile("([0-9a-f]{8})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]+)");
  
      private final Collection<Path> dataDirectories;
-     private final Map<String, TableSnapshot.Builder> snapshots = new 
HashMap<>();
  
      public SnapshotLoader()
      {
@@@ -75,103 -74,101 +74,118 @@@
          this.dataDirectories = dataDirs;
      }
  
 +    public SnapshotLoader(Directories directories)
 +    {
 +        
this(directories.getCFDirectories().stream().map(File::toPath).collect(Collectors.toList()));
 +    }
 +
+     @VisibleForTesting
+     static class Visitor extends SimpleFileVisitor<Path>
+     {
+         private static final Pattern UUID_PATTERN = 
Pattern.compile("([0-9a-f]{8})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]{4})([0-9a-f]+)");
+         private final Map<String, TableSnapshot.Builder> snapshots;
+ 
+         public Visitor(Map<String, TableSnapshot.Builder> snapshots)
+         {
+             this.snapshots = snapshots;
+         }
+ 
+         @Override
+         public FileVisitResult visitFileFailed(Path file, IOException exc) 
throws IOException
+         {
+             // Cassandra can remove some files while traversing the tree,
+             // for example when SSTables are compacted while we are walking 
it.
+             // SnapshotLoader is interested only in SSTables in snapshot 
directories which are not compacted,
+             // but we need to cover these in regular table directories too.
+             // If listing failed but exception is NoSuchFileException, then we
+             // just skip it and continue with the listing.
+             if (exc instanceof NoSuchFileException)
+                 return FileVisitResult.CONTINUE;
+             else
+                 throw exc;
+         }
+ 
+         @Override
+         public FileVisitResult preVisitDirectory(Path subdir, 
BasicFileAttributes attrs)
+         {
+             if 
(subdir.getParent().getFileName().toString().equals(SNAPSHOT_SUBDIR))
+             {
+                 logger.trace("Processing directory " + subdir);
+                 Matcher snapshotDirMatcher = 
SNAPSHOT_DIR_PATTERN.matcher(subdir.toString());
+                 if (snapshotDirMatcher.find())
+                 {
+                     try
+                     {
+                         loadSnapshotFromDir(snapshotDirMatcher, subdir);
+                     }
+                     catch (Throwable e)
+                     {
+                         logger.warn("Could not load snapshot from {}.", 
subdir, e);
+                     }
+                 }
+                 return FileVisitResult.SKIP_SUBTREE;
+             }
+ 
+             return 
subdir.getFileName().toString().equals(Directories.BACKUPS_SUBDIR)
+                    ? FileVisitResult.SKIP_SUBTREE
+                    : FileVisitResult.CONTINUE;
+         }
+ 
+         /**
+          * Given an UUID string without dashes (ie. 
c7e513243f0711ec9bbc0242ac130002)
+          * return an UUID object (ie. c7e51324-3f07-11ec-9bbc-0242ac130002)
+          */
+         static UUID parseUUID(String uuidWithoutDashes) throws 
IllegalArgumentException
+         {
+             assert uuidWithoutDashes.length() == 32 && 
!uuidWithoutDashes.contains("-");
+             String dashedUUID = 
UUID_PATTERN.matcher(uuidWithoutDashes).replaceFirst("$1-$2-$3-$4-$5");
+             return UUID.fromString(dashedUUID);
+         }
+ 
+         private void loadSnapshotFromDir(Matcher snapshotDirMatcher, Path 
snapshotDir)
+         {
+             String keyspaceName = snapshotDirMatcher.group("keyspace");
+             String tableName = snapshotDirMatcher.group("tableName");
+             UUID tableId = parseUUID(snapshotDirMatcher.group("tableId"));
+             String tag = snapshotDirMatcher.group("tag");
+             String snapshotId = buildSnapshotId(keyspaceName, tableName, 
tableId, tag);
+             TableSnapshot.Builder builder = 
snapshots.computeIfAbsent(snapshotId, k -> new 
TableSnapshot.Builder(keyspaceName, tableName, tableId, tag));
+             builder.addSnapshotDir(new File(snapshotDir));
+         }
+     }
+ 
 -    public Set<TableSnapshot> loadSnapshots()
 +    public Set<TableSnapshot> loadSnapshots(String keyspace)
      {
 +        // if we supply a keyspace, the walking max depth will be suddenly 
shorther
 +        // because we are one level down in the directory structure
 +        int maxDepth = keyspace == null ? 5 : 4;
++
+         Map<String, TableSnapshot.Builder> snapshots = new HashMap<>();
+         Visitor visitor = new Visitor(snapshots);
+ 
          for (Path dataDir : dataDirectories)
          {
 +            if (keyspace != null)
 +                dataDir = dataDir.resolve(keyspace);
 +
              try
              {
                  if (new File(dataDir).exists())
-                 {
-                     Files.walkFileTree(dataDir, Collections.emptySet(), 
maxDepth, this);
-                 }
 -                    Files.walkFileTree(dataDir, Collections.emptySet(), 5, 
visitor);
++                    Files.walkFileTree(dataDir, Collections.emptySet(), 
maxDepth, visitor);
                  else
-                 {
                      logger.debug("Skipping non-existing data directory {}", 
dataDir);
-                 }
              }
              catch (IOException e)
              {
                  throw new RuntimeException(String.format("Error while loading 
snapshots from %s", dataDir), e);
              }
          }
+ 
          return 
snapshots.values().stream().map(TableSnapshot.Builder::build).collect(Collectors.toSet());
      }
 +
 +    public Set<TableSnapshot> loadSnapshots()
 +    {
 +        return loadSnapshots(null);
 +    }
- 
-     @Override
-     public FileVisitResult visitFileFailed(Path file, IOException exc) throws 
IOException {
-         // Cassandra can remove some files while traversing the tree,
-         // for example when SSTables are compacted while we are walking it.
-         // SnapshotLoader is interested only in SSTables in snapshot 
directories which are not compacted,
-         // but we need to cover these in regular table directories too.
-         // If listing failed but such file exists and the exception is not 
NoSuchFileException, then we
-         // have a legitimate error while traversing the tree, otherwise just 
skip it and continue with the listing.
-         if (Files.exists(file) && !(exc instanceof NoSuchFileException))
-             return super.visitFileFailed(file, exc);
-         else
-             return FileVisitResult.CONTINUE;
-     }
- 
-     @Override
-     public FileVisitResult preVisitDirectory(Path subdir, BasicFileAttributes 
attrs)
-     {
-         if 
(subdir.getParent().getFileName().toString().equals(SNAPSHOT_SUBDIR))
-         {
-             logger.trace("Processing directory " + subdir);
-             Matcher snapshotDirMatcher = 
SNAPSHOT_DIR_PATTERN.matcher(subdir.toString());
-             if (snapshotDirMatcher.find())
-             {
-                 try
-                 {
-                     loadSnapshotFromDir(snapshotDirMatcher, subdir);
-                 } catch (Throwable e)
-                 {
-                     logger.warn("Could not load snapshot from {}.", subdir, 
e);
-                 }
-             }
-             return FileVisitResult.SKIP_SUBTREE;
-         }
- 
-         return 
subdir.getFileName().toString().equals(Directories.BACKUPS_SUBDIR)
-                ? FileVisitResult.SKIP_SUBTREE
-                : FileVisitResult.CONTINUE;
-     }
- 
-     private void loadSnapshotFromDir(Matcher snapshotDirMatcher, Path 
snapshotDir)
-     {
-         String keyspaceName = snapshotDirMatcher.group("keyspace");
-         String tableName = snapshotDirMatcher.group("tableName");
-         UUID tableId = parseUUID(snapshotDirMatcher.group("tableId"));
-         String tag = snapshotDirMatcher.group("tag");
-         String snapshotId = buildSnapshotId(keyspaceName, tableName, tableId, 
tag);
-         TableSnapshot.Builder builder = snapshots.computeIfAbsent(snapshotId, 
k -> new TableSnapshot.Builder(keyspaceName, tableName, tableId, tag));
-         builder.addSnapshotDir(new File(snapshotDir));
-     }
- 
-     /**
-      * Given an UUID string without dashes (ie. 
c7e513243f0711ec9bbc0242ac130002)
-      * return an UUID object (ie. c7e51324-3f07-11ec-9bbc-0242ac130002)
-      */
-     protected static UUID parseUUID(String uuidWithoutDashes) throws 
IllegalArgumentException
-     {
-         assert uuidWithoutDashes.length() == 32 && 
!uuidWithoutDashes.contains("-");
-         String dashedUUID = 
UUID_PATTERN.matcher(uuidWithoutDashes).replaceFirst("$1-$2-$3-$4-$5");
-         return UUID.fromString(dashedUUID);
-     }
  }
diff --cc src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
index 9c9e7f13f2,8e630c73ff..3925f3f9dc
--- a/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
+++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java
@@@ -106,11 -109,9 +109,14 @@@ public class SnapshotManager 
          }
      }
  
-     @VisibleForTesting
-     protected synchronized void loadSnapshots()
++    public synchronized Set<TableSnapshot> loadSnapshots(String keyspace)
++    {
++        return snapshotLoader.loadSnapshots(keyspace);
++    }
++
+     public synchronized Set<TableSnapshot> loadSnapshots()
      {
-         SnapshotLoader loader = new 
SnapshotLoader(DatabaseDescriptor.getAllDataFileLocations());
-         addSnapshots(loader.loadSnapshots());
+         return snapshotLoader.loadSnapshots();
      }
  
      @VisibleForTesting
@@@ -147,15 -148,11 +153,14 @@@
          }
      }
  
 +    /**
 +     * Deletes snapshot and remove it from manager
 +     */
-     public void clearSnapshot(TableSnapshot snapshot)
+     public synchronized void clearSnapshot(TableSnapshot snapshot)
      {
          for (File snapshotDir : snapshot.getDirectories())
-         {
              
Directories.removeSnapshotDirectory(DatabaseDescriptor.getSnapshotRateLimiter(),
 snapshotDir);
-         }
+ 
          expiringSnapshots.remove(snapshot);
      }
  
diff --cc 
test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
index eeb3b63fc3,edbfff2519..f9233bba8a
--- a/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
+++ b/test/unit/org/apache/cassandra/service/snapshot/SnapshotManagerTest.java
@@@ -50,24 -57,31 +57,30 @@@ public class SnapshotManagerTes
      @ClassRule
      public static TemporaryFolder temporaryFolder = new TemporaryFolder();
  
-     private TableSnapshot generateSnapshotDetails(String tag, Instant 
expiration, boolean ephemeral) throws Exception {
-         return new TableSnapshot(
-         "ks",
-         "tbl",
-         UUID.randomUUID(),
-         tag,
-         Instant.EPOCH,
-         expiration,
-         createFolders(temporaryFolder),
-         ephemeral
-         );
 -    private TableSnapshot generateSnapshotDetails(String tag, Instant 
expiration)
++    private TableSnapshot generateSnapshotDetails(String tag, Instant 
expiration, boolean ephemeral)
+     {
+         try
+         {
+             return new TableSnapshot("ks",
+                                      "tbl",
+                                      UUID.randomUUID(),
+                                      tag,
+                                      Instant.EPOCH,
+                                      expiration,
 -                                     createFolders(temporaryFolder));
++                                     createFolders(temporaryFolder),
++                                     ephemeral);
+         }
+         catch (Exception ex)
+         {
+             throw new RuntimeException(ex);
+         }
      }
  
 -
      @Test
 -    public void testLoadSnapshots() throws Exception
 -    {
 -        TableSnapshot expired = generateSnapshotDetails("expired", 
Instant.EPOCH);
 -        TableSnapshot nonExpired = generateSnapshotDetails("non-expired", 
now().plusSeconds(ONE_DAY_SECS));
 -        TableSnapshot nonExpiring = generateSnapshotDetails("non-expiring", 
null);
 +    public void testLoadSnapshots() throws Exception {
 +        TableSnapshot expired = generateSnapshotDetails("expired", 
Instant.EPOCH, false);
 +        TableSnapshot nonExpired = generateSnapshotDetails("non-expired", 
now().plusSeconds(ONE_DAY_SECS), false);
 +        TableSnapshot nonExpiring = generateSnapshotDetails("non-expiring", 
null, false);
          List<TableSnapshot> snapshots = Arrays.asList(expired, nonExpired, 
nonExpiring);
  
          // Create SnapshotManager with 3 snapshots: expired, non-expired and 
non-expiring
@@@ -146,21 -160,48 +159,66 @@@
          }
      }
  
 +    @Test
 +    public void testClearSnapshot() throws Exception
 +    {
 +        // Given
 +        SnapshotManager manager = new SnapshotManager(1, 3);
 +        TableSnapshot expiringSnapshot = generateSnapshotDetails("snapshot", 
now().plusMillis(50000), false);
 +        manager.addSnapshot(expiringSnapshot);
 +        assertThat(manager.getExpiringSnapshots()).contains(expiringSnapshot);
 +        assertThat(expiringSnapshot.exists()).isTrue();
 +
 +        // When
 +        manager.clearSnapshot(expiringSnapshot);
 +
 +        // Then
 +        
assertThat(manager.getExpiringSnapshots()).doesNotContain(expiringSnapshot);
 +        assertThat(expiringSnapshot.exists()).isFalse();
 +    }
- }
++
+     @Test // see CASSANDRA-18211
+     public void testConcurrentClearingOfSnapshots() throws Exception
+     {
+ 
+         AtomicReference<Long> firstInvocationTime = new AtomicReference<>(0L);
+         AtomicReference<Long> secondInvocationTime = new 
AtomicReference<>(0L);
+ 
+         SnapshotManager manager = new SnapshotManager(0, 5)
+         {
+             @Override
+             public synchronized void clearSnapshot(TableSnapshot snapshot)
+             {
+                 if (snapshot.getTag().equals("mysnapshot"))
+                 {
+                     firstInvocationTime.set(currentTimeMillis());
+                     Uninterruptibles.sleepUninterruptibly(10, SECONDS);
+                 }
+                 else if (snapshot.getTag().equals("mysnapshot2"))
+                 {
+                     secondInvocationTime.set(currentTimeMillis());
+                 }
+                 super.clearSnapshot(snapshot);
+             }
+         };
+ 
 -        TableSnapshot expiringSnapshot = 
generateSnapshotDetails("mysnapshot", Instant.now().plusSeconds(15));
++        TableSnapshot expiringSnapshot = 
generateSnapshotDetails("mysnapshot", Instant.now().plusSeconds(15), false);
+         manager.addSnapshot(expiringSnapshot);
+ 
+         manager.resumeSnapshotCleanup();
+ 
 -        Thread nonExpiringSnapshotCleanupThred = new Thread(() -> 
manager.clearSnapshot(generateSnapshotDetails("mysnapshot2", null)));
++        Thread nonExpiringSnapshotCleanupThred = new Thread(() -> 
manager.clearSnapshot(generateSnapshotDetails("mysnapshot2", null, false)));
+ 
+         // wait until the first snapshot expires
+         await().pollInterval(1, SECONDS)
+                .pollDelay(0, SECONDS)
+                .timeout(1, MINUTES)
+                .until(() -> firstInvocationTime.get() > 0);
+ 
+         // this will block until the first snapshot is cleaned up
+         nonExpiringSnapshotCleanupThred.start();
+         nonExpiringSnapshotCleanupThred.join();
+ 
+         assertTrue(secondInvocationTime.get() - firstInvocationTime.get() > 
10_000);
+     }
+ }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to