-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Update VReplication Timestamp w/ Heartbeat #6635
Changes from all commits
50bfb67
36f8b09
dc5e60d
9318042
2e4b5e9
68b0d74
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -458,8 +458,16 @@ func expectDBClientQueries(t *testing.T, queries []string) { | |
continue | ||
} | ||
var got string | ||
heartbeatRe := regexp.MustCompile(`update _vt.vreplication set time_updated=\d+, transaction_timestamp=\d+ where id=\d+`) | ||
retry: | ||
select { | ||
case got = <-globalDBQueries: | ||
// We rule out heartbeat time update queries because otherwise our query list | ||
// is indeterminable and varies with each test execution. | ||
if heartbeatRe.MatchString(got) { | ||
goto retry | ||
} | ||
|
||
var match bool | ||
if query[0] == '/' { | ||
result, err := regexp.MatchString(query[1:], got) | ||
|
@@ -493,6 +501,7 @@ func expectDBClientQueries(t *testing.T, queries []string) { | |
func expectNontxQueries(t *testing.T, queries []string) { | ||
t.Helper() | ||
failed := false | ||
heartbeatRe := regexp.MustCompile(`update _vt.vreplication set time_updated=\d+, transaction_timestamp=\d+ where id=\d+`) | ||
for i, query := range queries { | ||
if failed { | ||
t.Errorf("no query received, expecting %s", query) | ||
|
@@ -503,7 +512,7 @@ func expectNontxQueries(t *testing.T, queries []string) { | |
select { | ||
case got = <-globalDBQueries: | ||
|
||
if got == "begin" || got == "commit" || got == "rollback" || strings.Contains(got, "update _vt.vreplication set pos") { | ||
if got == "begin" || got == "commit" || got == "rollback" || strings.Contains(got, "update _vt.vreplication set pos") || heartbeatRe.MatchString(got) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps the query gets lower cased beforehand? Otherwise, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These were already here so I assume whoever built this had some wisdom with writing this catch. I believe that we always write them as lowercase (in vplayer), so this is safe. It's also just testing logic so if it breaks at some point, whoever broke it (by changing the casing that vplayer writes queries with) could adjust it then to fit their needs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any risk at all for some query to contain the text There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good question. I'm not sure why the original author of this test helper used Contains. It might be in an effort to strip potential whitespace? Or potentially to strip leading |
||
goto retry | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly make the query case insensitive? Granted,
_vt.vreplication
is case sensitive, but all other elements, keywords & columns, are not.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case we are the ones writing the heartbeat queries and they are always written as lower case. If there actually was a query written where that wasn't the case, it would be a sure sign that it was not a heartbeat update query. Therefore, I think it being case sensitive is a better catch method.