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: remove obsolete encryption-at-rest code #74314

Merged
merged 1 commit into from
Feb 3, 2022

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented Dec 29, 2021

Remove encryption-at-rest code that handles the upgrade process to the 21.2
encryption-at-rest formats and all code that reads the 21.1 and earlier
formats.

Fix #72520.

Release note: None

@jbowens jbowens requested review from a team December 29, 2021 18:54
@jbowens jbowens requested a review from a team as a code owner December 29, 2021 18:54
@jbowens jbowens requested review from adityamaru and removed request for a team December 29, 2021 18:54
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jbowens jbowens requested review from a team and shermanCRL and removed request for adityamaru, a team and shermanCRL December 29, 2021 18:55
@jbowens jbowens force-pushed the ear-cleanup branch 3 times, most recently from bf546e3 to a56b813 Compare December 29, 2021 20:22
Copy link
Collaborator Author

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/storage/pebble_file_registry.go, line 397 at r1 (raw file):

			}
			r.mu.entries[update.Filename] = update.Entry
		}

This logic is moved from the enginepb.FileRegistry type, because we no longer need the indirection of holding a enginepb.FileRegistry on PebbleFileRegistry. Instead we maintain just the map[string]*enginepb.FileEntry map directly.

@jbowens jbowens force-pushed the ear-cleanup branch 4 times, most recently from ba4bd75 to a75489b Compare December 30, 2021 01:02
@jbowens
Copy link
Collaborator Author

jbowens commented Dec 30, 2021

Will need to rebase once #74270 lands.

@postamar postamar removed the request for review from a team January 4, 2022 14:00
Copy link
Collaborator

@sumeerbhola sumeerbhola left a comment

Choose a reason for hiding this comment

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

:lgtm:

I didn't look carefully at the testdata files

Reviewed 6 of 14 files at r1, 1 of 5 files at r2, 9 of 10 files at r3.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @itsbilal and @jbowens)


pkg/ccl/storageccl/engineccl/pebble_key_manager.go, line 239 at r3 (raw file):

			return err
		}
	}

// Else there is no DataKeysRegistry file yet.


pkg/storage/pebble_file_registry.go, line 397 at r1 (raw file):

Previously, jbowens (Jackson Owens) wrote…

This logic is moved from the enginepb.FileRegistry type, because we no longer need the indirection of holding a enginepb.FileRegistry on PebbleFileRegistry. Instead we maintain just the map[string]*enginepb.FileEntry map directly.

ack

Remove encryption-at-rest code that handles the upgrade process to the 21.2
encryption-at-rest formats and all code that reads the 21.1 and earlier
formats.

Fix cockroachdb#72520.

Release note: None
Copy link
Collaborator Author

@jbowens jbowens left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r=sumeerbhola

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @itsbilal, @jbowens, and @sumeerbhola)


pkg/ccl/storageccl/engineccl/pebble_key_manager.go, line 239 at r3 (raw file):

Previously, sumeerbhola wrote…

// Else there is no DataKeysRegistry file yet.

Done.

@craig
Copy link
Contributor

craig bot commented Feb 3, 2022

Build succeeded:

@craig craig bot merged commit 0b16926 into cockroachdb:master Feb 3, 2022
@jbowens jbowens deleted the ear-cleanup branch February 3, 2022 21:48
jbowens added a commit to jbowens/cockroach that referenced this pull request Feb 8, 2022
Remove the migration server DeprecateBaseEncryptionRegistry RPC. It was
used for the migration of the encryption-at-rest registry format in
21.2 and is no longer relevant.

Missed this in cockroachdb#74314.

Release note: None
craig bot pushed a commit that referenced this pull request Feb 9, 2022
75870: rfc: pgwire-compatible query cancellation r=otan,knz a=rafiss

refs #41335

Release note: None

76007: physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy, regr_intercept, regr_r2, and regr_slope aggregate functions. r=yuzefovich a=mneverov

physicalplan: add support for multi-stage execution of regr_avgx, regr_avgy,
regr_intercept, regr_r2, and regr_slope aggregate functions.

See #58347.

Release note (performance improvement): regr_avgx, regr_avgy, regr_intercept,
regr_r2, and regr_slope aggregate functions are now evaluated more efficiently
in a distributed setting

76115: sql: use 16 as default bucket count for hash index r=chengxiong-ruan a=chengxiong-ruan

for some reason I forgot to modify it in my previous pr :(

Release note (sql change): 16 is used as the default bucket count
for hash sharded index.

76262: server: remove DeprecateBaseEncryptionRegistry r=jbowens a=jbowens

Remove the migration server DeprecateBaseEncryptionRegistry RPC. It was
used for the migration of the encryption-at-rest registry format in
21.2 and is no longer relevant.

Missed this in #74314.

Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Max Neverov <[email protected]>
Co-authored-by: Chengxiong Ruan <[email protected]>
Co-authored-by: Jackson Owens <[email protected]>
RajivTS pushed a commit to RajivTS/cockroach that referenced this pull request Mar 6, 2022
Remove the migration server DeprecateBaseEncryptionRegistry RPC. It was
used for the migration of the encryption-at-rest registry format in
21.2 and is no longer relevant.

Missed this in cockroachdb#74314.

Release note: None
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.

storage: remove monolith encryption-at-rest registry
3 participants