pnowojski commented on code in PR #26566:
URL: https://github.com/apache/flink/pull/26566#discussion_r2093312783
##########
flink-state-backends/flink-statebackend-rocksdb/src/main/java/org/apache/flink/state/rocksdb/RocksDBKeyedStateBackendBuilder.java:
##########
@@ -440,6 +441,13 @@ public RocksDBKeyedStateBackend<K> build() throws
BackendBuildingException {
nativeMetricMonitor,
manualCompactionManager);
} catch (Throwable e) {
+ boolean cancelled = cancelStreamRegistry.isClosed();
+ // log ASAP because close can block or fail too
+ if (cancelled) {
+ logger.info("RocksDB state backend build cancelled");
Review Comment:
Thanks for the feedback. I think your points correct, but that wouldn't be
an easy and quick change.
Also the impact wouldn't be quite low of such changes. Errors/exceptions in
this code are happening very rarely, while cancelation happens many more orders
of magnitude more frequently. For any single subtask failures, many more
subtasks are cancelled for the same job (due to parallelism and number of
tasks). Also failures from this part of the code are happening also quite
rarely.
And even if they happen, the failure message doesn't matter that much, as
the exception cause itself.
Anyway, this is a bit of out of scope for this cancellation fix. If you
think something can be improved with logging here (ideally with some motivating
case), please create a ticket and you can also create a contribution.
--
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]
