Skip to content

kvserver: fix TestEagerReplication#61847

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
lunevalex:alex/fix-test-eager-replication
Mar 15, 2021
Merged

kvserver: fix TestEagerReplication#61847
craig[bot] merged 1 commit intocockroachdb:masterfrom
lunevalex:alex/fix-test-eager-replication

Conversation

@lunevalex
Copy link
Copy Markdown

Fixes #54646

PR #61644 introduced a bug into the TestEagerReplication test.
Using a single TestServer sets the default zone config to require
only one replica, which means the split in this test does not
trigger a replication attempt. This PR sets the PartOfCluster parameter
on the TestServer to disable that behavior.

Release note: None

Fixes cockroachdb#54646

PR cockroachdb#61644 introduced a bug into the TestEagerReplication test.
Using a single TestServer sets the default zone config to require
only one replica, which means the split in this test does not
trigger a replication attempt. This PR sets the PartOfCluster parameter
on the TestServer to disable that behavior.

Release note: None
@lunevalex lunevalex requested a review from tbg March 11, 2021 15:57
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Member

@tbg tbg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @lunevalex)

@lunevalex
Copy link
Copy Markdown
Author

TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Mar 15, 2021

Build succeeded:

@craig craig bot merged commit 1bec46e into cockroachdb:master Mar 15, 2021
lunevalex pushed a commit to lunevalex/cockroach that referenced this pull request Mar 23, 2021
PR cockroachdb#61847 fixed the flake in TestEagerReplication but was not
rebased with master, so the skipped tag was not properly removed.
This PR actually unskips TestEagerReplication.

Release note: None
craig bot pushed a commit that referenced this pull request Mar 23, 2021
62377: changefeedccl: allow topic_name parameter for changefeed kafka sinks r=[stevendanna,miretskiy] a=HonoreDB

Previously, changes for a table went to a Kafka topic
named for that table, with users only able to specify a prefix.
Some users, however, need changes to go to a specific topic,
including sometimes the same one for more than one table,
distinguishing messages using metadata.
This patch allows the `?topic_name=foo` parameter to be added
to Kafka sink URIs. This will override the per-table topic
generation, so that changes for every table will all go to
the specified topic. It may be used in conjunction with
`topic_prefix`, although the distinction is not meaningful.

Release note (enterprise change): Kafka sink URIs now accept
the "topic_name" parameter to override per-table topic names.

Closes #59300
Closes #58302


62414: workload/schemachange: add SURVIVE syntax r=ajwerner a=otan

see individual commits for details

62420: cliccl/load: fix TestLoadShowIncremental typo r=pbardea a=Elliebababa

This patch fixs typo of TestLoadShowIncremental.

Resolves: #62416

Release note: none

62465: kvccl: re-order enterprise check in canSendToFollower r=nvanbenschoten a=nvanbenschoten

Fixes #62447.

In #62447, Erik found that #59571 had re-ordered the call to
`utilccl.CheckEnterpriseEnabled` to occur before checking the batch in
`canSendToFollower`, instead of after. This added an error allocation
into the hot path of all reads, which showed up in CPU profiles and
caused an 8% performance regression on `kv95`. This commit fixes this by
moving the enterprise check back out of the hot-path for all non-stale
read-only batches.

A follow up to this PR would be to make `utilccl.CheckEnterpriseEnabled`
cheaper by avoiding the error allocation for callers that don't need an
error. This work is not done in this commit.

62473: kvserver: unskip TestEagerReplication r=lunevalex a=lunevalex

PR #61847 fixed the flake in TestEagerReplication but was not
rebased with master, so the skipped tag was not properly removed.
This PR actually unskips TestEagerReplication.

Release note: None

Co-authored-by: Aaron Zinger <zinger@cockroachlabs.com>
Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Co-authored-by: elliebababa <ellie24.huang@gmail.com>
Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Alex Lunev <alexl@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kv/kvserver: TestEagerReplication failed

3 participants