htran1 commented on a change in pull request #2722: GOBBLIN-865: Add feature that enables PK-chunking in partition URL: https://github.com/apache/incubator-gobblin/pull/2722#discussion_r326307389
########## File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceExtractor.java ########## @@ -869,7 +862,7 @@ public SalesforceBulkJobId getQueryResultIdsPkChunking(String entity, List<Predi // wait for completion, failure, or formation of PK chunking batches while ((bulkBatchInfo.getState() != BatchStateEnum.Completed) && (bulkBatchInfo.getState() != BatchStateEnum.Failed) - && (!usingPkChunking || bulkBatchInfo.getState() != BatchStateEnum.NotProcessed)) { + && (bulkBatchInfo.getState() != BatchStateEnum.NotProcessed)) { Review comment: Is skipping over this state safe for the non-pk chunking case? Could it be a valid waiting state for non-pk chunking queries? I think we should raise an error if there are any NotProcessed batches and if there are more than one NotProcessed batches for PK chunking mode since that indicates some expected work was not performed and there may be data loss if we move the checkpoint forward. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services