opt: race in AssignPlaceholders #34904
Assignees
Labels
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
C-test-failure
Broken test (automatically or manually discovered).
O-roachtest
O-robot
Originated from a bot.
Milestone
Comments
cockroach-teamcity
added
C-test-failure
|
nvanbenschoten
added
the
C-bug
|
@andy-kimball looks like access to the |
|
It shouldn't need to be synchronized. I suspect this is related to query caching. +@RaduBerinde |
|
@nvanbenschoten how did you get that log? I'd like to look at the other goroutines |
|
I think I found the problem - the |
|
I will work on this. The fix should be easy, modulo writing a test. |
RaduBerinde
changed the title
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 16, 2019
Test that reproduces cockroachdb#34904. Writing the test exposed a problem in the `CopyAndReplace` API: there was no way to recursively copy a subtree when creating an internal node. The API was reverted to the first iteration Andy had in his PR. Release note: None
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 16, 2019
When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. Fixes cockroachdb#34904. Release note (bug fix): Fixed a crash related to cached plans.
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 19, 2019
Test that reproduces cockroachdb#34904. Writing the test exposed a problem in the `CopyAndReplace` API: there was no way to recursively copy a subtree when creating an internal node. The API was reverted to the first iteration Andy had in his PR. Release note: None
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 19, 2019
When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. Fixes cockroachdb#34904. Release note (bug fix): Fixed a crash related to cached plans.
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 19, 2019
When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. Fixes cockroachdb#34904. Release note (bug fix): Fixed a crash related to cached plans.
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 20, 2019
Test that reproduces cockroachdb#34904. Writing the test exposed a problem in the `CopyAndReplace` API: there was no way to recursively copy a subtree when creating an internal node. The API was reverted to the first iteration Andy had in his PR. Release note: None
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 20, 2019
When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. For statistics, we simply clear out the annotation because it can be recalculated as needed (and it turns out to be faster than copying it). Fixes cockroachdb#34904. Release note (bug fix): Fixed a crash related to cached plans.
RaduBerinde
added a commit
to RaduBerinde/cockroach
that referenced
this issue
Feb 20, 2019
When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. For statistics, we simply clear out the annotation because it can be recalculated as needed (and it turns out to be faster than copying it). Fixes cockroachdb#34904. Release note (bug fix): Fixed a crash related to cached plans.
craig bot
pushed a commit
that referenced
this issue
Feb 21, 2019
35027: opt: fix race caused by shared table annotations r=RaduBerinde a=RaduBerinde #### opt: test reproducing detached memo race Test that reproduces #34904. Writing the test exposed a problem in the `CopyAndReplace` API: there was no way to recursively copy a subtree when creating an internal node. The API was reverted to the first iteration Andy had in his PR. Release note: None #### opt: replace ColumnMeta.TableMeta with a TableID The TableMeta field is problematic: it needs to be fixed up when copying Metadata; and since it points into a slice that we append to, it may or may not be aliased with the corresponding entry in `tables`. Avoiding these complication by just storing the TabeID. The `md` backpointer is also removed and QualifiedAlias is moved to Metadata. Release note: None #### opt: fix race caused by shared table annotations When we assign placeholders on a detached memo, we copy the metadata. However, this does a shallow copy of table annotations; this is problematic for the statistics annotation which is a mutable object. The fix is to register a copy function for each type of table annotation as part of `NewTableAnnID`. Fixes #34904. Release note (bug fix): Fixed a crash related to cached plans. Co-authored-by: Radu Berinde <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
SHA: https://github.com/cockroachdb/cockroach/commits/acba091f04f3d8ecabf51009bf394951fbd3643c
Parameters:
To repro, try:
Failed test: https://teamcity.cockroachdb.com/viewLog.html?buildId=1137872&tab=buildLog
The text was updated successfully, but these errors were encountered: