[
https://issues.apache.org/jira/browse/FLINK-8057?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16256629#comment-16256629
]
Kostas Kloudas commented on FLINK-8057:
---------------------------------------
I agree that this check is never failing but if it ever fails, then there is
something wrong.
So I would recommend to keep the check, as I consider a good practice to be
defensive and have a check for every possible execution path (as long as it is
not in the critical path), and just change the exception message to sth that
indicates that execution took an unexpected path.
What about sth along the lines: kvStateId + " appears registered although it
should not." ?
> KvStateRegistry#registerKvState may randomly fail
> -------------------------------------------------
>
> Key: FLINK-8057
> URL: https://issues.apache.org/jira/browse/FLINK-8057
> Project: Flink
> Issue Type: Improvement
> Components: Queryable State
> Affects Versions: 1.4.0
> Reporter: Chesnay Schepler
> Assignee: Kostas Kloudas
>
> The {{KvStateRegistry#registerKvState}} method is a bit weird. On each call,
> a new KvStateID is generated, and we then check whether a state for this ID
> was already registered. Realistically, this is never the case making the
> check unnecessary, and on the off-chance that it does happen the job will
> fail for no good reason.
> {code}
> KvStateID kvStateId = new KvStateID();
> if (registeredKvStates.putIfAbsent(kvStateId, kvState) == null) {
> KvStateRegistryListener listener = this.listener.get();
> if (listener != null) {
> listener.notifyKvStateRegistered(
> jobId,
> jobVertexId,
> keyGroupRange,
> registrationName,
> kvStateId);
> }
> return kvStateId;
> } else {
> throw new IllegalStateException(kvStateId + " is already registered.");
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
