sjyang18 commented on pull request #5241:
URL: https://github.com/apache/nifi/pull/5241#issuecomment-914736679


   > @sjyang18 Thanks for these changes. Overall it looks good. Other than 
stuff like how the log messages are handled and finals I think there are a few 
things to look at:
   > 
   > * I think the various inserts could be simplified. We have 
`insertWithTransactionalBatch()`, `insertRecord()`, 
`chooseInsertMethodAndRun()`, and `bulkInsert()`. I think it gets a little 
confusing. I think `insertRecord()` and `insertWithTransactionalBatch()` could 
be combined if there were a branch with `if (records.size() == 1)`.
   > * We need to handle more error cases. In particular 413 (Entity too large).
   > * I don't totally understand the yield handling in `onTrigger()`. Can you 
explain that a little more?
   
   To me, yield handing, thus putting the current flowfile into the queue for 
retry, seems to make sense when server-side throttling is happening and 
server-side RU change is in progress.  For other errors, I output the flowfile 
to failure relationship.


-- 
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]


Reply via email to