abjfh30 commented on PR #29488: URL: https://github.com/apache/flink/pull/29488#issuecomment-6368648672
One thing I noticed while working on this, which I deliberately left out of this PR: `clearResourceFor()` releases the owner's resources with a plain `forEach`. If one `release()` throws - `TieredStorageMemoryManagerImpl.release()` rethrows on an `awaitTermination` failure, for example - the remaining resources of that owner are never released. The owner has already been removed from the map at that point, so there is no retry, and the exception also propagates through `ResultPartition.release()` and `ResultPartitionManager.releasePartition()`, which holds the `registeredPartitions` lock while calling it, so it can cut short the release of the other partitions in the same batch as well. This is pre-existing behaviour and not caused by the thread-safety fix, so I kept it out of this change. Would you prefer a separate JIRA for it, or should it be handled here as an additional commit (release each resource in a try/catch, log the failure and continue with the rest)? -- 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]
