remove support for prefix replacement #3405
Conversation
RaduBerinde
requested review from
dt,
msbutler,
a team and
aadityasondhi
and removed request for
aadityasondhi
|
This change is |
RaduBerinde
force-pushed
the
no-prefix-repl
branch
2 times, most recently
from
f4855ad to
a7318cd
Compare
internal/manifest/version.go
Outdated
| @@ -865,14 +861,14 @@ func (m *FileMetadata) Validate(cmp Compare, formatKey base.FormatKey) error { | |||
| return base.CorruptionErrorf("file metadata FileBacking not set") | |||
| } | |||
|
|
|||
| if m.PrefixReplacement != nil { | |||
| if m.SyntheticPrefix.IsSet() { | |||
| if !m.Virtual { | |||
| return base.CorruptionErrorf("prefix replacement rule set with non-virtual file") | |||
There was a problem hiding this comment.
uber nit: these error messages could be updated to use the phrase "synthetic prefix" instead of "prefix replacement"
internal/manifest/version_edit.go
Outdated
| @@ -385,18 +385,19 @@ func (v *VersionEdit) Decode(r io.Reader) error { | |||
| } | |||
|
|
|||
| case customTagPrefixRewrite: | |||
| // We used to have a content prefix; we no longer use it. | |||
There was a problem hiding this comment.
why can't we completely delete this content prefix logic?
|
Previously, msbutler (Michael Butler) wrote…
We can, but everybody who is currently running experiments will have to wipe their stores.. Seemed like keeping that extra byte would be fine. We could also add a new tag and then retire this tag a bit later, not sure it's worth the trouble though. |
Remove `PrefixReplacement` in favor of `SyntheticPrefix`. The general prefix replacement turns out to be too difficult so we will instead strip the prefix when writing out the backup files.
RaduBerinde
force-pushed
the
no-prefix-repl
branch
from
a7318cd to
5c1d056
Compare
There was a problem hiding this comment.
TFTR!
Reviewable status: 0 of 21 files reviewed, 2 unresolved discussions (waiting on @dt and @msbutler)
internal/manifest/version.go line 866 at r1 (raw file):
Previously, msbutler (Michael Butler) wrote…
uber nit: these error messages could be updated to use the phrase "synthetic prefix" instead of "prefix replacement"
Done.
internal/manifest/version_edit.go line 388 at r1 (raw file):
Previously, RaduBerinde wrote…
We can, but everybody who is currently running experiments will have to wipe their stores.. Seemed like keeping that extra byte would be fine.
We could also add a new tag and then retire this tag a bit later, not sure it's worth the trouble though.
Ok I cleaned this up as discussed offline.
Remove
PrefixReplacementin favor ofSyntheticPrefix. The general prefix replacement turns out to be too difficult so we will instead strip the prefix when writing out the backup files.