yifan-c commented on code in PR #31:
URL: 
https://github.com/apache/cassandra-analytics/pull/31#discussion_r1456714473


##########
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/data/CassandraDataLayer.java:
##########
@@ -341,24 +341,31 @@ private CompletionStage<Map<String, AvailabilityHint>> 
createSnapshot(RingRespon
                     LOGGER.info("Creating snapshot on instance snapshotName={} 
keyspace={} table={} datacenter={} fqdn={}",
                                 snapshotName, maybeQuotedKeyspace, 
maybeQuotedTable, datacenter, ringEntry.fqdn());
                     SidecarInstance sidecarInstance = new 
SidecarInstanceImpl(ringEntry.fqdn(), sidecarClientConfig.effectivePort());
-                    createSnapshotFuture = sidecar
-                                           .createSnapshot(sidecarInstance, 
maybeQuotedKeyspace, maybeQuotedTable, snapshotName)
-                                           .handle((resp, throwable) -> {
-                                               if (throwable == null)
-                                               {
-                                                   // Create snapshot succeeded
-                                                   return hint;
-                                               }
-
-                                               if (isExhausted(throwable))
-                                               {
-                                                   LOGGER.warn("Failed to 
create snapshot on instance", throwable);
-                                                   return 
PartitionedDataLayer.AvailabilityHint.DOWN;
-                                               }
-
-                                               LOGGER.error("Unexpected error 
creating snapshot on instance", throwable);
-                                               return 
PartitionedDataLayer.AvailabilityHint.UNKNOWN;
-                                           });
+                    String resolvedSnapshotTtl = options.clearSnapshot() ? 
options.effectiveSnapshotTtl() : null;
+                    if (!options.clearSnapshot() && 
options.userProvidedSnapshotTtl() != null)
+                    {
+                        LOGGER.warn("Snapshot TTL option was provided along 
with Clear Snapshot option, bulk reader" +
+                                    "can honor only one of the 2. Clear 
Snapshot takes precedence over Snapshot TTL, " +
+                                    "hence snapshot {} will not be removed 
after the specified snapshot TTL option", snapshotName);
+                    }

Review Comment:
   The condition is reversed too. Beside that, I think the warning is best 
emitted from `ClientConfig`, and only use the resolved value at the call-sites. 



-- 
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: commits-unsubscr...@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to