Skip to content

systemconfigwatcher: remove NewWithAdditionalProvider#141815

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:remove-sys-config-gossip
Feb 25, 2025
Merged

systemconfigwatcher: remove NewWithAdditionalProvider#141815
craig[bot] merged 1 commit intocockroachdb:masterfrom
rafiss:remove-sys-config-gossip

Conversation

@rafiss
Copy link
Copy Markdown
Collaborator

@rafiss rafiss commented Feb 21, 2025

This was needed in the 21.2-22.1 mixed version state, before tenants had a spanconfig reconciliation job. Now that the job exists, there's no need to pass in an additional provider for tenants.

This takes the work in #82329 a bit further.

informs #76625
Release note: None

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@rafiss rafiss force-pushed the remove-sys-config-gossip branch 2 times, most recently from e7823a4 to 8d14b80 Compare February 21, 2025 17:11
@rafiss rafiss force-pushed the remove-sys-config-gossip branch 2 times, most recently from 49de4fe to 59a3b25 Compare February 21, 2025 17:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 21, 2025

⚪ Sysbench [SQL, 3node, oltp_read_write]
Metric Old Commit New Commit Delta Note Threshold
sec/op 11.07m ±0% 11.05m ±1% ~ p=0.529 n=10 3.0%
errs/op 0.000 ±0% 0.000 ±0% ~ p=1.000 n=10 0.0%
allocs/op 10.18k ±1% 10.15k ±1% ~ p=0.149 n=10 2.0%
B/op 2.204Mi ±0% 2.201Mi ±0% ~ p=0.247 n=10 2.0%
Reproduce

benchdiff binaries:

mkdir -p benchdiff/37b40b8/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/37b40b8f8a387a59b65f2d5fab39d2ebda39df1c/bin/pkg_sql_tests benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/2e66bf7/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/2e66bf7c1c1e67eaa11c2994380d467224863cb3/bin/pkg_sql_tests benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/SQL/3node/oltp_read_write$ --old=2e66bf7 --new=37b40b8 ./pkg/sql/tests
🟡 Sysbench [KV, 1node, local, oltp_read_only]
Metric Old Commit New Commit Delta Note Threshold
🟡 sec/op 663.2µ ±1% 668.0µ ±1% +0.73% p=0.043 n=10 2.0%
errs/op 0.000 ±0% 0.000 ±0% ~ p=1.000 n=10 0.0%
allocs/op 439.0 ±0% 439.0 ±0% ~ p=1.000 n=10 1.5%
B/op 254.2Ki ±0% 254.1Ki ±0% ~ p=0.668 n=10 1.5%
Reproduce

benchdiff binaries:

mkdir -p benchdiff/37b40b8/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/37b40b8f8a387a59b65f2d5fab39d2ebda39df1c/bin/pkg_sql_tests benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/2e66bf7/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/2e66bf7c1c1e67eaa11c2994380d467224863cb3/bin/pkg_sql_tests benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/KV/1node_local/oltp_read_only$ --old=2e66bf7 --new=37b40b8 ./pkg/sql/tests
🟡 Sysbench [KV, 1node, local, oltp_write_only]
Metric Old Commit New Commit Delta Note Threshold
🟡 sec/op 1.348m ±0% 1.355m ±1% +0.54% p=0.035 n=10 2.5%
errs/op 0.000 ±0% 0.000 ±0% ~ p=1.000 n=10 0.0%
allocs/op 1.395k ±0% 1.396k ±0% ~ p=0.077 n=10 1.8%
B/op 290.1Ki ±0% 290.3Ki ±0% ~ p=0.579 n=10 1.8%
Reproduce

benchdiff binaries:

mkdir -p benchdiff/37b40b8/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/37b40b8f8a387a59b65f2d5fab39d2ebda39df1c/bin/pkg_sql_tests benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/37b40b8/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
mkdir -p benchdiff/2e66bf7/bin/1058449141
gcloud storage cp gs://cockroach-microbench-ci/builds/2e66bf7c1c1e67eaa11c2994380d467224863cb3/bin/pkg_sql_tests benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests
chmod +x benchdiff/2e66bf7/bin/1058449141/cockroachdb_cockroach_pkg_sql_tests

benchdiff command:

benchdiff --run=^BenchmarkSysbench/KV/1node_local/oltp_write_only$ --old=2e66bf7 --new=37b40b8 ./pkg/sql/tests
Artifacts

download:

mkdir -p new
gcloud storage cp gs://cockroach-microbench-ci/artifacts/37b40b8f8a387a59b65f2d5fab39d2ebda39df1c/13462853539-1/\* new/
mkdir -p old
gcloud storage cp gs://cockroach-microbench-ci/artifacts/2e66bf7c1c1e67eaa11c2994380d467224863cb3/13462853539-1/\* old/
Legend
  • Neutral: No significant performance change.
  • 🟡 Warning: Slight degradation, likely due to variance, but still within thresholds.
  • 🔴 Regression: Likely performance regression, requiring investigation.
  • 🟢 Improvement: Possible performance gain.

No regressions detected!

built with commit: 37b40b8f8a387a59b65f2d5fab39d2ebda39df1c

This was needed in the 21.2-22.1 mixed version state, before tenants had
a spanconfig reconciliation job. Now that the job exists, there's no
need to pass in an additional provider for tenants.

Release note: None
@rafiss rafiss force-pushed the remove-sys-config-gossip branch from 59a3b25 to 37b40b8 Compare February 21, 2025 18:22
@rafiss rafiss marked this pull request as ready for review February 21, 2025 19:01
@rafiss rafiss requested review from a team as code owners February 21, 2025 19:01
@rafiss rafiss requested review from a team, arulajmani and dhartunian and removed request for a team February 21, 2025 19:01
Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @dhartunian)

Copy link
Copy Markdown
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-02-24 at 10 59 46 🙏 Thank you for cleaning this up!

@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Feb 25, 2025

tftr!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Feb 25, 2025

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Feb 25, 2025

Build failed:

@rafiss
Copy link
Copy Markdown
Collaborator Author

rafiss commented Feb 25, 2025

bors retry

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Feb 25, 2025

@craig craig bot merged commit 19290ae into cockroachdb:master Feb 25, 2025
24 checks passed
@rafiss rafiss deleted the remove-sys-config-gossip branch February 25, 2025 22:43
craig bot pushed a commit that referenced this pull request Mar 5, 2025
141849: server,kvtenant: remove usages of gossip.KeyDeprecatedSystemConfig r=rafiss a=rafiss

followup after #141815

This key was only needed in the 21.2/22.2 mixed version state.

Removing it and the related machinery allows us to make
kvtenant.connector stop implementing SystemConfigProvider.

The key should not be reused until it's entirely deleted from the gossip
network of existing clusters, so for now the string constant remains as
a placeholder.

fixes #76625
Release note: None


Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants