nussknacker
nussknacker copied to clipboard
Fixup to async DefaultAsyncExecutionConfigPreparer
Fixup to async DefaultAsyncExecutionConfigPreparer - using class loader release hook instead of counting prepareExecutionContext/close method calls:
- Closed ExecutorService could leak into execution after restart because lack of synchronization in close method (cached reference to Some in asyncExecutionContext var for thread)
- prepare/close counter could fall below zero and lead to odd behaviour in cases when close was called without a prior open method call (e.g. in case of operator initializeState error - look into org.apache.flink.streaming.runtime.tasks.RegularOperatorChain.initializeStateAndOpenOperators and org.apache.flink.runtime.taskmanager.Task.restoreAndInvoke)
Checklist before merge
- [ ] Added entry in Changelog.md describing the change from the perspective of a public distribution user
- [ ] Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
- [ ] Verify that PR will be squashed during merge
created: #5007 :warning: Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.
I'm still thinking about enabling this mechanism to work in our tests, because unfortunately there is a slightly different classloader arrangement in them, so DefaultAsyncExecutionConfigPreparer is not loaded in FlinkUserCodeClassLoader but in AppClassLoader
fixed in https://github.com/TouK/nussknacker/pull/6055 and https://github.com/TouK/nussknacker/pull/6204