workload/schemachanger: re-enable the schema changer workload #88085
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
fqazi
changed the title
fqazi
force-pushed
the
insertFix
branch
3 times, most recently
from
666d255 to
525d162
Compare
fqazi
changed the title
Previously, when resolve OID ran into errors on dropped descriptors we would surface internal inactive descriptor errors to the caller. This was inadequate because these errors would not be properly ignored leading to unexpected failures, which did not exist on 22.1. To address this, this patch adds logic to ignore inactive descriptor errors like we would have in the past, returning that no OID has been resolved. Release note: None
Previously, when we refactored support for how statement generation and errors, we did not correctly update support for potential execution errors. As a result support for inserts / add foreign key constraints was regressed. This path, fixes the support for possible execution errors in both cases. Release note: None Release Justification: Low changes to re-enable an existing test case
ajwerner
reviewed
Sep 19, 2022
There was a problem hiding this comment.
It's still flakey due to at least two other issues I saw:
- Something about max command size for inverted index entries
- Integer overflows due to expressions
The second one popped up more regularly. Consider you have a table:
CREATE TABLE (i INT, INDEX ((i+i)); If i is large enough that 2*i overflows int64, there's a problem.
Reviewable status:
complete! 0 of 0 LGTMs obtained
|
@ajwerner For (2) We did have generated expression validation (see validateGeneratedExpressionsForInsert), but some of those errors were in the potential error sets. Let me re-run it a few more times to confirm to make sure it's fully squashed. |
|
I'll see if I can reproduce (1) and address that as well in a separate commit |
fqazi
force-pushed
the
insertFix
branch
2 times, most recently
from
fa1863e to
be3557e
Compare
Previously, when a CTAS statement was executed we did not correctly detect if the SELECT portion could fail on generated columns. This could lead to test failures, even though the observed errors are correct. To address, this patch will execute the select independently to validate any generated column errors. Release note: None
fqazi
force-pushed
the
insertFix
branch
4 times, most recently
from
af2c072 to
6ea5120
Compare
Previously, the watch dog threads inside the schema changer workload could dump the stacks way incorrectly, since it didn't give connections time to terminate. This change adapts the watch dog logic to have a delay. Release note: None
Fixes: cockroachdb#87615 Previously, this test was no longer marked as a release blocker because of how unstable it was. With the latest set of fixes we expect this to be far more stable. Release note: None
ajwerner
approved these changes
Sep 22, 2022
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 3 of 3 files at r2, 2 of 2 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status:
|
@ajwerner TFTR! bors r+ |
|
Build succeeded: |
This was referenced Sep 22, 2022
|
I see that this was merged directly into release-22.2 (kinda surprised bors worked). Is it also being added to master? |
|
@rafiss Yes, it will be merged into master separately. |
craig bot
pushed a commit
that referenced
this pull request
Oct 28, 2022
88537: workload/schemachanger: re-enable the schema changer workload r=fqazi a=fqazi Fixes: #82133 Previously the schema changer workload was disabled because of two intermittent issues: The workload itself had an issue due to a recent refactor that did not properly detect/handle potential execution errors. While the set of errors was generated it was never used properly in some cases after a recent refactor Some scenarios involving inserts could fail binding OID's inside the resolveOID code path, specifically this code path does not properly handle dropped descriptors. These changes enforces the old behaviour where the resolution should just fail. Note: This is the same as the previously merged PR (#88085 on 22.2) Co-authored-by: Faizan Qazi <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the schema changer workload was disabled because of two intermittent issues: