frankgh commented on code in PR #31: URL: https://github.com/apache/cassandra-analytics/pull/31#discussion_r1456715644
########## 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; Review Comment: I think the condition here is correct. When clear snapshot is set, it allows the snapshot TTL to be set. When clear snapshot is set to false, we don't want the TTL to clear the snapshot at a later point. I think that's how I understand it -- 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