Conversation
lucasbru
left a comment
There was a problem hiding this comment.
Thanks for the PR! I made a pass
| * for how this is determined, and for why a member the coordinator has not heard from reads as processing. | ||
| * @param caughtUp | ||
| * Whether the member has restored the task to within {@code acceptable.recovery.lag}. | ||
| * Should only be checked if the task is not {@code restoring}. |
There was a problem hiding this comment.
I think this doc has it backwards - caughtUp is only meaningful while restoring is true (once a member stops restoring it stops reporting offsets for the task, so caughtUp would read as false). hot() relies on the short-circuit !restoring || caughtUp to be correct today, but as written this would mislead anyone who reads caughtUp on its own.
| } else if (holder == null && onDisk(currentAssignment, targetProcessId, task)) { | ||
| // The target owner's process left this task's state on disk and can reopen it. How far behind that | ||
| // state is cannot be measured -- a member reports an end offset only for a task it is restoring -- | ||
| // so this takes precedence over promoting a caught-up copy, because the common way a task ends up |
There was a problem hiding this comment.
Is that right? For a caught up copy, we know that we are not behind, for the on disk task, we don't.
There was a problem hiding this comment.
Yes, it's an optimistic approach here.
| currentOwner = Optional.of(holder.memberId()); | ||
| } else if (holder == null && onDisk(currentAssignment, targetProcessId, task)) { | ||
| // The target owner's process left this task's state on disk and can reopen it. How far behind that | ||
| // state is cannot be measured -- a member reports an end offset only for a task it is restoring -- |
There was a problem hiding this comment.
Another member could report the end offset (because it's actively restoring). This is actually likely to be the case. Could we measure the lag that way?
If a member leaves or drops out of a group, or a rebalance happens while an active task is still restoring, it's possible that the task is moved to a different member which might imply a cold start, because the old member is gone or does not do any processing yet, so putting a warmup on the new owner is useless. However, if there is another member holding a hot standby, we can temporarily promote it to an active task, and put a warmup task on the new owner, for a proper staging and no offline time.
07c278d to
4d235e4
Compare
If a member leaves or drops out of a group, or a rebalance happens while
an active task is still restoring, it's possible that the task is moved
to a different member which might imply a cold start, because the old
member is gone or does not do any processing yet, so putting a warmup on
the new owner is useless.
However, if there is another member holding a hot standby, we can
temporarily promote it to an active task, and put a warmup task on the
new owner, for a proper staging and no offline time.
Part of KIP-1071.