storage,kv: extend MVCC metamorphic tests to cover range keys #82545
Assignees
Labels
A-storage
Relating to our storage engine (Pebble) on-disk storage.
C-enhancement
Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
T-storage
Storage Team
Comments
nicktrav
added
C-enhancement
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jul 6, 2022
This change adds MVCCDeleteRangeUsingTombstone to the MVCC metamorphic tests. MVCCDeleteRange had already existed in this test suite, so this ended up being a relatively simple addition. One part of cockroachdb#82545, with possibly more parts to follow as other MVCC-level operations are added that utilize `writer.{Put,Clear}{MVCC,Engine}RangeKey`. Release note: None.
itsbilal
added a commit
to itsbilal/cockroach
that referenced
this issue
Jul 7, 2022
This change adds MVCCDeleteRangeUsingTombstone to the MVCC metamorphic tests. MVCCDeleteRange had already existed in this test suite, so this ended up being a relatively simple addition. One part of cockroachdb#82545, with possibly more parts to follow as other MVCC-level operations are added that utilize `writer.{Put,Clear}{MVCC,Engine}RangeKey`. Release note: None.
craig bot
pushed a commit
that referenced
this issue
Jul 7, 2022
83875: opt: fix memo cycle caused by join ordering r=DrewKimball a=DrewKimball
In some rare cases when null-rejection rules fail to fire, a redundant filter
can be inferred in an `InnerJoin` - `LeftJoin` complex. This could previously
result in the `JoinOrderBuilder` attempting to add a `Select` to the same memo
group as its input, which would create a memo cycle. This patch prevents the
`JoinOrderBuilder` from adding the `Select` to the memo in such cases.
What follows is a more detailed explanation of the conditions that could
previously cause a memo cycle.
`InnerJoin` operators have two properties that make them more 'reorderable'
than other types of joins: (1) their conjuncts can be reordered separately
and (2) new conjuncts can be inferred from equalities. As a special case of
(1), an `InnerJoin` can be pushed into the left side of a `LeftJoin`, leaving
behind any `Select` conjuncts that reference the right side of the `LeftJoin`.
This allows the `JoinOrderBuilder` to make the following transformation:
```
(InnerJoin
A
(InnerJoin
B
(LeftJoin
C
D
c=d
)
b=c
)
a=b, a=d
)
=>
(InnerJoin
A
(Select
(LeftJoin
(InnerJoin
B
C
b=c
)
D
c=d
)
b=d // Inferred filter!
)
a=b, a=d
)
```
Note the new `b=d` filter that was inferred and subsequently left on
a `Select` operator after the reordering. The crucial point is that
this filter is redundant - the input to the `Select` is already a
valid reordering of the `BCD` join complex.
The `JoinOrderBuilder` avoids adding redundant filters to `InnerJoin`
operators, but does not do the same for the `Select` case because it
was assumed that the filters left on the `Select` would never be redundant.
This is because the `a=d` filter *should* rejects nulls, so the `LeftJoin`
should have been simplified. However, in rare cases null-rejection does not
take place. Because the input to the `Select` is already a valid reordering,
the `JoinOrderBuilder` ends up trying to add the `Select` to the same group
as its input - namely, the `BCD` join group. This causes a cycle in the memo.
Fixes #80901
Release note (bug fix): Fixed a bug that could cause an optimizer
panic in rare cases when a query had a left join in the input of
an inner join.
83945: storage/metamorphic: Add MVCC delete range using tombstone r=erikgrinaker a=itsbilal
This change adds MVCCDeleteRangeUsingTombstone to the MVCC
metamorphic tests. MVCCDeleteRange had already existed in
this test suite, so this ended up being a relatively simple
addition.
One part of #82545, with possibly more parts to follow
as other MVCC-level operations are added that utilize
`writer.{Put,Clear}{MVCC,Engine}RangeKey`.
Release note: None.
83992: server: log stacks on 500 errors r=ajwerner a=ajwerner
Before this change, we'd just log the error body, which often is not very
helpful. The format specifier makes me think that the intention was to log
the stacks. This made debugging [this](#83677 (comment))
trivial as opposed to hard.
Release note: None
84015: bazel: clear configurations when running `git grep` in `check.sh` r=miretskiy,mari-crl a=rickystewart
The configurations `grep.{column,lineNumber,fullName}` can be set
globally or on a per-user basis and change the output of `git grep`,
which breaks checks that do exact-string matching. We manually clear
these configurations for the `git grep`s in this file to ensure the
output is predictable on any machine.
Release note: None
Co-authored-by: Andrew Kimball <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Is your feature request related to a problem? Please describe.
We currently have a metamorphic test at the KV layer, which runs nightly. With the addition of range keys (as part of the work for #70429), we'll want to enhance the metamorphic test harness to ensure that we have coverage of MVCC operations that make use of range keys.
This will no doubt prove invaluable in detecting edge-cases that would otherwise be very tricky to write tests for, or even dream up in the first place!
Describe the solution you'd like
Extend the existing MVCC metamorphic tests to cover operations that use Pebble range keys.
Jira issue: CRDB-16537
Epic CRDB-2624
The text was updated successfully, but these errors were encountered: