-
Notifications
You must be signed in to change notification settings - Fork 195
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Only log that the scan is complete one time for s3 scan #3168
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,7 @@ public class S3ScanPartitionCreationSupplier implements Function<Map<String, Obj | |
private static final String BUCKET_OBJECT_PARTITION_KEY_FORMAT = "%s|%s"; | ||
static final String SCAN_COUNT = "SCAN_COUNT"; | ||
static final String LAST_SCAN_TIME = "LAST_SCAN_TIME"; | ||
static final String SINGLE_SCAN_COMPLETE = "SINGLE_SCAN_COMPLETE"; | ||
|
||
private final S3Client s3Client; | ||
private final BucketOwnerProvider bucketOwnerProvider; | ||
|
@@ -143,6 +144,7 @@ private boolean isKeyMatchedBetweenTimeRange(final LocalDateTime lastModifiedTim | |
|
||
private void initializeGlobalStateMap(final Map<String, Object> globalStateMap) { | ||
globalStateMap.put(SCAN_COUNT, 0); | ||
globalStateMap.put(SINGLE_SCAN_COMPLETE, false); | ||
} | ||
|
||
private boolean isLastModifiedTimeAfterMostRecentScanForBucket(final String bucketName, | ||
|
@@ -174,14 +176,22 @@ private Instant getMostRecentLastModifiedTimestamp(final ListObjectsV2Response l | |
} | ||
|
||
private boolean shouldScanBeSkipped(final Map<String, Object> globalStateMap) { | ||
|
||
if (Objects.isNull(schedulingOptions) && hasAlreadyBeenScanned(globalStateMap)) { | ||
LOG.info("Skipping scan because the buckets have already been scanned once"); | ||
|
||
if (!(Boolean) globalStateMap.get(SINGLE_SCAN_COMPLETE)) { | ||
LOG.info("Single S3 scan has already been completed"); | ||
globalStateMap.put(SINGLE_SCAN_COMPLETE, true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference between There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SCAN_COUNT could block scanning since the global state doesn't go away. so it's better to keep it separate for this log |
||
} | ||
|
||
return true; | ||
} | ||
|
||
if (Objects.nonNull(schedulingOptions) && | ||
(hasReachedMaxScanCount(globalStateMap) || !hasReachedScheduledScanTime(globalStateMap))) { | ||
|
||
|
||
|
||
if (hasReachedMaxScanCount(globalStateMap)) { | ||
LOG.info("Skipping scan as the max scan count {} has been reached", schedulingOptions.getCount()); | ||
} else { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why log only during the seconds attempt at scanning? Why not log once the first scan is complete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could log after the first scan, but with source coordination this will only get run again after all the scanned objects have been processed, so as far as timing of the log it makes more since when trying to scan again