Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check keyspace snapshot time if none specified for backup restores #13557

Merged
merged 7 commits into from
Jul 27, 2023
Merged

check keyspace snapshot time if none specified for backup restores #13557

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5f444f1
check keyspace snapshot time if none specified for backup restores
pbibra Jul 19, 2023
a2e9eb5
update recovery test to use custom restore time
pbibra Jul 24, 2023
480e459
fmt recovery.go
pbibra Jul 25, 2023
4ccd55d
fix test
pbibra Jul 25, 2023
a6c5a1c
update comments
pbibra Jul 25, 2023
3cb7e61
fmt
pbibra Jul 25, 2023
28d2fc9
add check for backup replica update in test
pbibra Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
add check for backup replica update in test 
Signed-off-by: Priya Bibra <pbibra@slack-corp.com>
  • Loading branch information
@pbibra
pbibra committed Jul 26, 2023
commit 28d2fc9e87306906006e452159eed16273c2dd63
22 changes: 22 additions & 0 deletions go/test/endtoend/recovery/unshardedrecovery/recovery.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,28 @@ func TestRecoveryImpl(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, "msgx1", qr.Rows[0][0].ToString())

// check that replica1, used for the backup, has the new value
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()

ticker := time.NewTicker(time.Second)
defer ticker.Stop()

for {
qr, err = replica1.VttabletProcess.QueryTablet("select msg from vt_insert_test where id = 1", keyspaceName, true)
assert.NoError(t, err)
if qr.Rows[0][0].ToString() == "msgx1" {
break
}

select {
case <-ctx.Done():
t.Error("timeout waiting for new value to be replicated on replica 1")
break
case <-ticker.C:
}
}

// take second backup of value = msgx1
err = localCluster.VtctlclientProcess.ExecuteCommand("Backup", replica1.Alias)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're missing validation that replica1 has the value "msgx1". The backup of replica1 might only contains "test1", which means the fact we restore and get "test1" does not indicate which backup we've restored.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should add that the way to resolve this is to run a context.WithTimeout() + time.Ticker select loop waiting for the data to appear in replica1. Our experience shows that in GitHub CI things can be unbelievably slow. I'd say give this a timeout of 10s.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated!

assert.NoError(t, err)
Expand Down