turcsanyip commented on PR #8013: URL: https://github.com/apache/nifi/pull/8013#issuecomment-1848993611
Thanks for your review @exceptionfactory! Based on your suggestion, I extracted the conversion methods to a utility class and also reorganized the methods a bit. Also found a bug with the retry logic that has been fixed. With retry backoff, the retry calls are executed asynchronously and the caller would not wait the result. Backoff is not really needed (it does not help on the race condition) so I simply removed it. I have one more open question: cleaning up the obsolete items from the state when the user changes the flow and configures e.g. a new event hub or consumer group on the processor. In this case the old ownership and checkpoint data remains persisted in the state currently. Clearing the state is the user's responsibility now. On the other hand, it also means that the user has the option to go back to the original settings and continue with those checkpoints. In general, I would opt for cleaning up the state after configuration changes and storing only the current checkpoints (state size, no outdated items). Unfortunately, there is no trivial way to clear the state (`StateManager.clear()` cannot be used due to the concurrent access in the cluster) and it was implemented without clean-up in the original PR so I did not change it. However, now it looks feasible to me and it may be worth implementing the clean-up logic. What is your opinion? -- 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]
