vttablet/tabletmanager: Update the tablet_externally_reparented_timestamp every time the TabletExternallyReparented RPC is called. #3009
Conversation
There was a problem hiding this comment.
This change looks good.
I'm also wondering if we should set TER time at this point (to time.Now()): https://github.com/youtube/vitess/blob/master/go/vt/vttablet/tabletmanager/init_tablet.go#L109
so that when the master tablet is restarted, and we realize it's the master, we're good right away, and don't have to wait for another TER. What do you think? (Can be a different PR).
|
Yep, I'll do that change in a separate PR. vtgate_buffer e2e test is failing with the new code. Looking into it now. |
…tamp every time the TabletExternallyReparented RPC is called. This was always the original intention, but until now we skipped setting the timestamp if the tablet is already the MASTER. See the change in rpc_external_reparent.go. I've wrote a minimal unit test to verify the new behavior. Additionally, I had to change the vtgate buffer code because it was dependent on the old behavior to detect a corner case when it should not start to buffer. That's when it saw the end of a reparent *before* an error due to the same reparent. In order to better test the new code, I had to change the unit tests to use a fake of time.Now(). BUG=63883255
michael-berlin
force-pushed
the
ter_no_skip
branch
from
20937df
to
c784c65
Compare
|
The failed e2e test covered a problem in the change: if currentMaster is nil, we should not set "lastReparent" and instead wait for the next time when currentMaster != nil and we actually see a change of the master. I fixed the unit test to cover this case by adding an initial StatsUpdate() call in the TestBuffer method. Tests are passing now. |
…oroutine leakage (vitessio#3009) * cherry pick of 13824 * Fix conflict Signed-off-by: Rohit Nayak <rohit@planetscale.com> --------- Signed-off-by: Rohit Nayak <rohit@planetscale.com> Co-authored-by: Rohit Nayak <rohit@planetscale.com>
This was always the original intention, but until now we skipped setting the timestamp if the tablet is already the MASTER. See the change in rpc_external_reparent.go.
I've wrote a minimal unit test to verify the new behavior.
Additionally, I had to change the vtgate buffer code because it was dependent on the old behavior to detect a corner case when it should not start to buffer. That's when it saw the end of a reparent before an error due to the same reparent. In order to better test the new code, I had to change the unit tests to use a fake of time.Now().