Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: fix TestEagerReplication #61847

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

lunevalex
Copy link
Collaborator

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
Member

This change is Reviewable

Copy link
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
Collaborator Author

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 15, 2021

Build succeeded:

@craig craig bot merged commit 1bec46e into cockroachdb:master Mar 15, 2021
lunevalex added 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 <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: elliebababa <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Alex Lunev <[email protected]>
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