turcsanyip commented on code in PR #8231:
URL: https://github.com/apache/nifi/pull/8231#discussion_r1450405484
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/ListS3.java:
##########
@@ -499,23 +500,19 @@ private void listByTrackingTimestamps(ProcessContext
context, ProcessSession ses
writer.beginListing();
do {
- VersionListing versionListing = bucketLister.listVersions();
+ final VersionListing versionListing =
bucketLister.listVersions();
for (S3VersionSummary versionSummary :
versionListing.getVersionSummaries()) {
- long lastModified =
versionSummary.getLastModified().getTime();
- if (lastModified < currentTimestamp
- || lastModified == currentTimestamp &&
currentKeys.contains(versionSummary.getKey())
- || (maxAgeMilliseconds != null && (lastModified <
(listingTimestamp - maxAgeMilliseconds)))
- || lastModified > (listingTimestamp -
minAgeMilliseconds)) {
+ final long lastModified =
versionSummary.getLastModified().getTime();
+ if (lastModified == currentTimestamp &&
currentKeys.contains(versionSummary.getKey())
+ || !includeObjectInListing(versionSummary,
listingTimestamp)) {
Review Comment:
`lastModified < currentTimestamp` check is still needed and cannot be
removed. _currentTimestamp_ here means the "latest listed timestamp in the
previous round" and it is used to filter out the already listed items.
```suggestion
if (lastModified < currentTimestamp
|| lastModified == currentTimestamp &&
currentKeys.contains(versionSummary.getKey())
|| !includeObjectInListing(versionSummary,
listingTimestamp)) {
```
##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/main/java/org/apache/nifi/processors/aws/s3/ListS3.java:
##########
@@ -321,6 +322,8 @@ public class ListS3 extends AbstractS3Processor implements
VerifiableProcessor {
private volatile boolean justElectedPrimaryNode = false;
private volatile boolean resetEntityTrackingState = false;
private volatile
ListedEntityTracker<ListableEntityWrapper<S3VersionSummary>>
listedEntityTracker;
+ private Long minObjectAgeMilliseconds;
+ private Long maxObjectAgeMilliseconds;
Review Comment:
Please add `volatile` for the fields (like the others above). It is used
because the variable is written and then read by different threads (written in
`onScheduled`, read in `onTrigger`).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
