Skip to content

fix data race in vtexplain implementation of ExecuteBatch#3831

Merged
demmer merged 2 commits intovitessio:masterfrom
tinyspeck:fix-vtexplain-race
Apr 13, 2018
Merged

fix data race in vtexplain implementation of ExecuteBatch#3831
demmer merged 2 commits intovitessio:masterfrom
tinyspeck:fix-vtexplain-race

Conversation

@demmer
Copy link
Copy Markdown
Member

@demmer demmer commented Apr 12, 2018

To prevent concurrent access to the BindVariables map, vtexplain
needs to copy the map when simulating RPCs from the vtgate to the
vttablet.

Fix a bug where the ExecuteBatch implementation didn't do this
quite right which caused a data race.

To prevent concurrent access to the BindVariables map, vtexplain
needs to copy the map when simulating RPCs from the vtgate to the
vttablet.

Fix a bug where the ExecuteBatch implementation didn't do this
quite right which caused a data race.

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>
// copy the bindVars into the executor to avoid a data race.
for _, query := range queries {
bindVariables := sqltypes.CopyBindVariables(query.BindVariables)
query.BindVariables = bindVariables
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

query.BindVariables = sqltypes.CopyBindVariables(query.BindVariables)
Go guarantees this is safe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated

Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>
@demmer
Copy link
Copy Markdown
Member Author

demmer commented Apr 13, 2018

Travis failures are unrelated -- merging in spite of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants