workload/tpcc: support replicated indexes and lease partitioning #36855
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 |
nvanbenschoten
force-pushed
the
nvanbenschoten/geoTpcc3
branch
2 times, most recently
from
3ebed10 to
1c9b8dd
Compare
ajwerner
approved these changes
Apr 19, 2019
There was a problem hiding this comment.
Neat. I did not do any meaningful inspection of the testdata files in opt. The workload code lgtm
Reviewed 6 of 6 files at r1, 4 of 4 files at r2, 1 of 1 files at r3, 6 of 6 files at r4, 3 of 3 files at r5, 1 of 1 files at r6, 2 of 2 files at r7, 1 of 1 files at r8, 2 of 2 files at r10.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/workload/tpcc/partition.go, line 264 at r7 (raw file):
// The stock_item_fk_idx can't be partitioned because it doesn't have a // warehouse prefix. It's an all-around unfortunate index that we only // need because of a restriction in SQL.
Just out of curiousity, what's the restriction in SQL?
andy-kimball
approved these changes
Apr 19, 2019
There was a problem hiding this comment.
opt changes
Reviewable status:
craig bot
pushed a commit
that referenced
this pull request
May 20, 2019
36854: workload/tpcc: improve indexing to permit better partitioning r=nvanbenschoten a=nvanbenschoten This PR contains a number of incremental improvements to TPC-C that allow it to be more effectively partitioned. The changes focus on migrating indexes to all be partitionable by warehouse and removing indexes that are only needed for foreign keys when fks are not enabled. This is all a precursor to a follow-up PR: #36855. I'm assigning @jordanlewis and @danhhz as reviewers because between the two of you most of the changes here have already been agreed upon. This PR will require me to regenerate all TPC-C fixtures. I intend to do so before it is merged. Co-authored-by: Nathan VanBenschoten <[email protected]>
nvanbenschoten
force-pushed
the
nvanbenschoten/geoTpcc3
branch
from
1c9b8dd to
93cdb43
Compare
The table is immutable, so we can use a "replicated index" configuration to ensure that all lookups into it are local and never cross zones. Release note: None
This breaks compatibility with 2.0 nodes, but that's far enough back that we don't care anymore. Release note: None
This commit adds support for two different partitioning strategies: replication partitioning and lease partitioning. The former is the only strategy we supported before and constrains replication for a given partition to within a single zone. The latter is new and collocates read leases for a given partition to within a single zone. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/geoTpcc3
branch
from
93cdb43 to
ebb68aa
Compare
nvanbenschoten
commented
May 20, 2019
There was a problem hiding this comment.
TFTRs!
bors r+
Reviewable status:
pkg/workload/tpcc/partition.go, line 264 at r7 (raw file):
Previously, ajwerner wrote…
Just out of curiousity, what's the restriction in SQL?
|
Bors didn't seem to pick this up. Let's try again. bors r+ |
craig bot
pushed a commit
that referenced
this pull request
May 20, 2019
36855: workload/tpcc: support replicated indexes and lease partitioning r=nvanbenschoten a=nvanbenschoten This PR adds support for a new geo-replicated configuration of TPC-C. Instead of partitioning replication, the PR allows us to partition leases while maintaining cross-region replication. It exposes this through a new `--partition-strategy` option, which can be set to `replication` or `leases`. The former is the only strategy we supported before and constrains replication for a given partition to within a single zone. The latter is new and collocates read leases for a given partition to within a single zone. In order to get optimal performance out of this configuration, the PR also switches to a "replicated index" configuration for the `item` table. This table is the only part of TPC-C that cannot be segmented on warehouse boundaries. Luckily, it is read-only, so replicated indexes are a perfect fit to reduce cross-zone communication. cc. @robert-s-lee I think this is a better configuration for testing geo-replicated TPC-C than the one we had before. Once this is merged I think it will make a great demo. Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
danhhz
added a commit
to danhhz/cockroach
that referenced
this pull request
May 22, 2019
In cockroachdb#36855, we bumped the workload-version of tpcc from 2.0.1 to 2.1.0. The IMPORT commands generated by `fixtures import` include the version built in so that all the nodes processing the IMPORT (which internally synthesize the data) can check against their internal version. This prevents nodes from generating incompatible data, but an unfortunate side effect of this is that the tpcc version in the workload binary must match the one in the cockroach nodes. This works for roachtests in master, but not for release branches, which use a workload binary built from master. Instead, use the `cockroach workload fixtures import` subcommand, which is guaranteed to match. Release note: None
craig bot
pushed a commit
that referenced
this pull request
May 22, 2019
37719: kv: finish and enable parallel commits r=nvanbenschoten a=nvanbenschoten The majority of this PR is addressing the final work item for parallel commits - splitting pre-commit QueryIntents from parallel committing EndTransaction requests. If a batch is performing a parallel commit and the transaction has previously pipelined writes that have yet to be proven successful then the EndTransaction request will be preceded by a series of QueryIntent requests (see txn_pipeliner.go). Before evaluating, each of these QueryIntent requests will grab latches and wait for their corresponding write to finish. This is how the QueryIntent requests synchronize with the write they are trying to verify. If these QueryIntents remained in the same batch as the EndTransaction request then they would force the EndTransaction request to wait for the previous write before evaluating itself. This "pipeline stall" would effectively negate the benefit of the parallel commit. To avoid this, we make sure that these "pre-commit" QueryIntent requests are split from and issued concurrently with the rest of the parallel commit batch. With this complete, parallel commits is then able to be enabled. ### Benchmarks One of my recent experiments was to run the `indexes` benchmark at varying numbers of secondary indexes on a 3-node geo-distributed cluster. Below are plots with and without parallel commits enabled. We can see that without parallel commits, the introduction of the first secondary index doubles p50 latency and reduces throughput (at a fixed concurrency) by 56%. With parallel commits, the introduction of the secondary index has almost no noticeable effect on p50 latency and reduces throughput by only 29%.   I expect I'll polish off more comparison benchmarks over the next few weeks to further demonstrate the effect of parallel commits. 37726: roachtest: fix `fixtures import` on release branches r=tbg,nvanbenschoten a=danhhz In #36855, we bumped the workload-version of tpcc from 2.0.1 to 2.1.0. The IMPORT commands generated by `fixtures import` include the version built in so that all the nodes processing the IMPORT (which internally synthesize the data) can check against their internal version. This prevents nodes from generating incompatible data, but an unfortunate side effect of this is that the tpcc version in the workload binary must match the one in the cockroach nodes. This works for roachtests in master, but not for release branches, which use a workload binary built from master. Instead, use the `cockroach workload fixtures import` subcommand, which is guaranteed to match. Release note: None Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Daniel Harrison <[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.
This PR adds support for a new geo-replicated configuration of TPC-C. Instead of partitioning replication, the PR allows us to partition leases while maintaining cross-region replication. It exposes this through a new
--partition-strategyoption, which can be set toreplicationorleases. The former is the only strategy we supported before and constrains replication for a given partition to within a single zone. The latter is new and collocates read leases for a given partition to within a single zone.In order to get optimal performance out of this configuration, the PR also switches to a "replicated index" configuration for the
itemtable. This table is the only part of TPC-C that cannot be segmented on warehouse boundaries. Luckily, it is read-only, so replicated indexes are a perfect fit to reduce cross-zone communication.cc. @robert-s-lee I think this is a better configuration for testing geo-replicated TPC-C than the one we had before. Once this is merged I think it will make a great demo.