tzulitai edited a comment on pull request #13761:
URL: https://github.com/apache/flink/pull/13761#issuecomment-717743321
@pnowojski @banmoy
Yes, I think it won't hurt to add checking the `isUsingCustomRawKeyedState`
flag to the snapshot path of legacy raw keyed state timers.
As of now, if users are already using raw keyed state, snapshotting the
timers (in the case of RocksDB + heap-based timers) would have already failed
with `IOException("Key group X already registered!")`.
Additionally checking `isUsingCustomRawKeyedState` on the snapshot path
gives us the opportunity to throw with a more meaningful error message. Note
that we can't just silently skip snapshotting timers, because that would result
in data loss; we have to throw and fail the snapshot in this case.
> Maybe we could go even one step further? throw new
UnsupportedOperationException() when operator with isUsingCustomRawKeyedState()
is trying to register a timer?
@pnowojski Not quite sure about this. Registering timers should work
regardless of the `isUsingCustomRawKeyedState` IF the user is not using the
RocksDB + heap-based timers configuration. Technically, it is possible to do
this check in the `InternalTimerServiceImpl`, I don't have a strong opinion on
this. @banmoy what do you think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
