Revert "opt: disallow mutations under union" #41356
Merged
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 reverts commit a34d705. Release justification: recently merge fix no longer needed (thanks to cockroachdb#41307). Release note (sql change): Mutations under UNION or UNION ALL are allowed again.
|
This change is |
|
LGTM |
|
hold up on this though cause maybe there's some problems with #41356
<#41356> - some Jepsen tests
started failing
…On Mon, Oct 7, 2019 at 10:54 AM Andrei Matei ***@***.***> wrote:
LGTM
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#41356?email_source=notifications&email_token=AAC4C4JUPJIUST64ZPKR5ILQNNER7A5CNFSM4I5ZFDNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAQUKDA#issuecomment-539051276>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC4C4J4AZ6BQ5DVS2FZKOLQNNER7ANCNFSM4I5ZFDNA>
.
|
|
Yeah, won't merge for now. |
|
merge ahead |
|
bors r+ |
craig bot
pushed a commit
that referenced
this pull request
Oct 10, 2019
41356: Revert "opt: disallow mutations under union" r=RaduBerinde a=RaduBerinde This reverts commit a34d705. Release justification: recently merge fix no longer needed (thanks to #41307). Release note (sql change): Mutations under UNION or UNION ALL are allowed again. 41358: sql/logictest: add logictest for all expected 1PC txn statements r=nvanbenschoten a=nvanbenschoten First commit from #41324. We expect a selection of simple single-statement mutations to hit the "1-phase commit" transaction fast-path. #41320 demonstrated how critical this is, as regressions here can cause major (> 50%) performance hits to benchmarks and user workloads. This commit adds a logic test to validate that these statements continue to hit this fast-path. Release justification: Testing only. 41371: kv: avoid allocations in client.Txn constructor r=nvanbenschoten a=nvanbenschoten This PR contains two small wins that help speed up the client.Txn constructor, which is responsible for **2.90%** of a CPU profile when running Sysbench's `oltp_point_select` workload. The biggest win here will come from addressing #32508. #### kv: lazily create *RangeIterator in txnPipeliner This allocation was responsible for **0.34%** of a CPU profile when running Sysbench's `oltp_point_select` workload. #### kv: only re-alloc refresh spans in augmentMetaLocked if necessary This was responsible for **0.057%** of a CPU profile when running Sysbench's `oltp_point_select` workload. Release justification: None. These can wait for the branch to split. 41372: sql/pgwire: statically allocate format code slice for all text args/cols r=nvanbenschoten a=nvanbenschoten This allocation was responsible for **0.8%** of a CPU profile when running Sysbench's oltp_point_select workload. Release justification: Probably none, although this does look very safe. Release note: None 41379: pkg/sql: use ring.Buffer in StmtBuf r=nvanbenschoten a=nvanbenschoten The StmtBuf has the exact access patterns typically associated with a ring buffer. Command instances are added to the back of the buffer and trimmed from the front of the buffer. These operations are often performed in lockstep, but that is not always the case, so we need the buffer to be able to grow. `ring.Buffer` provides exactly these semantics and it allows us to avoid memory allocations on each Command in `StmtBuf.Push`. Release justification: None. Don't merge until branch is split. Co-authored-by: Radu Berinde <radu@cockroachlabs.com> Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Build succeeded |
craig bot
pushed a commit
that referenced
this pull request
Oct 14, 2019
41500: release-19.2: Revert "opt: disallow mutations under union" r=RaduBerinde a=RaduBerinde Backport 1/1 commits from #41356. /cc @cockroachdb/release --- This reverts commit a34d705. Release justification: recently merged fix no longer needed (thanks to #41307). Release note (sql change): Mutations under UNION or UNION ALL are allowed again. Co-authored-by: Radu Berinde <radu@cockroachlabs.com>
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.
This reverts commit a34d705.
Release justification: recently merge fix no longer needed (thanks
to #41307).
Release note (sql change): Mutations under UNION or UNION ALL are
allowed again.