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

storage: move shared storage instantiation code to CCL #114267

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

itsbilal
Copy link
Contributor

@itsbilal itsbilal commented Nov 10, 2023

This change moves the shared storage factory
instantiation code to pkg/storageccl/engineccl/, as well as
rditer.IterateReplicaKeySpansShared. This change also
checks for an enterprise license before doing a fast rebalance.

Fixes #114185.

Epic: none

Release note: None

@itsbilal itsbilal requested review from a team as code owners November 10, 2023 21:15
@itsbilal itsbilal requested review from herkolategan, renatolabs and jbowens and removed request for a team November 10, 2023 21:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@itsbilal
Copy link
Contributor Author

@williamkulju I wonder if the license requirement is rigid; looking at the code for encryption-at-rest, we instantiate it in CCL code but we don't force an enterprise license check for that. This PR does a license check too for disaggregated storage, but just wanted to confirm if that's necessary from a product standpoint.

@itsbilal itsbilal force-pushed the disagg-license-check branch from 43e0920 to 8d77f47 Compare November 21, 2023 21:34
@itsbilal itsbilal requested a review from a team November 21, 2023 21:34
Copy link

blathers-crl bot commented Nov 21, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@itsbilal itsbilal changed the title storage: add license check for shared storage storage: move shared storage instantiation code to CCL Nov 21, 2023
@itsbilal itsbilal force-pushed the disagg-license-check branch 4 times, most recently from eeccfe3 to 6f830f6 Compare November 27, 2023 21:18
Copy link
Member

@RaduBerinde RaduBerinde 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 10 of 10 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @herkolategan, @jbowens, and @renatolabs)

This change moves the shared storage factory
instantiation code to pkg/storageccl/engineccl/, as well as
rditer.IterateReplicaKeySpansShared. This change also
checks for an enterprise license before doing a fast rebalance.

Fixes cockroachdb#114185.

Epic: none

Release note: None
@itsbilal itsbilal force-pushed the disagg-license-check branch from 6f830f6 to eebda9e Compare November 28, 2023 19:52
@itsbilal itsbilal added the backport-23.2.x Flags PRs that need to be backported to 23.2. label Nov 28, 2023
@itsbilal
Copy link
Contributor Author

TFTR!

bors r=RaduBerinde

@craig
Copy link
Contributor

craig bot commented Nov 28, 2023

Build failed (retrying...):

@craig
Copy link
Contributor

craig bot commented Nov 28, 2023

Build succeeded:

@craig craig bot merged commit a2316a9 into cockroachdb:master Nov 28, 2023
8 checks passed
Copy link

blathers-crl bot commented Nov 28, 2023

Encountered an error creating backports. Some common things that can go wrong:

  1. The backport branch might have already existed.
  2. There was a merge conflict.
  3. The backport branch contained merge commits.

You might need to create your backport manually using the backport tool.


error setting reviewers, but backport branch blathers/backport-release-23.2-114267 is ready: POST https://api.github.com/repos/cockroachdb/cockroach/pulls/115218/requested_reviewers: 422 Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the cockroachdb/cockroach repository. []

Backport to branch 23.2.x failed. See errors above.


🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

itsbilal added a commit to itsbilal/cockroach that referenced this pull request Nov 29, 2023
Since cockroachdb#114267, disaggregated storage rebalance
requires an enterprise license to happen quickly.
This change updates the disagg-rebalance roachtest
to pass in a license.

Epic: none

Release note: None
craig bot pushed a commit that referenced this pull request Nov 29, 2023
113960: workload/schemachange: instrument PGX r=rafiss a=chrisseto

#### f5df1cd workload/schemachange: instrument PGX

This commit instruments the `MultiConnPool` used in the schemachange
workload with OTeL by implementing  `pgx.QueryTracer`.

Doing so ensures that all queries executed by the workload are logged to
spans. Future commits will instrument the workers and query helpers. The
combination of all these instrumentations will allow debuggers to easily
inspect all SQL queries emitted by the workload and isolate them to
individual workers and transactions.

Epic: CRDB-19168
Release note: None

115270: roachtest: require license in disagg-rebalance r=jbowens a=itsbilal

Since #114267, disaggregated storage rebalance
requires an enterprise license to happen quickly.
This change updates the disagg-rebalance roachtest to pass in a license.

Epic: none

Release note: None

Co-authored-by: Chris Seto <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
blathers-crl bot pushed a commit that referenced this pull request Nov 29, 2023
Since #114267, disaggregated storage rebalance
requires an enterprise license to happen quickly.
This change updates the disagg-rebalance roachtest
to pass in a license.

Epic: none

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.2.x Flags PRs that need to be backported to 23.2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gate Disagg Storage code in CCL
3 participants