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?
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]
