Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Remove bzip2, gzip, and none from supported compression types
Browse files Browse the repository at this point in the history
These extra compression types are inferior for us and have caused us
extra support burden (especially none).

This change does retain the ArchiveFormat::Tar such that we can continue
to skip compression for unit tests. However, "tar" has been removed from
the possible values array which means passing "--snapshot-archive-format
tar" will error out.
Steven Czabaniuk committed Sep 28, 2023
1 parent fa168e3 commit bd3d1fa
Showing 9 changed files with 10 additions and 93 deletions.
2 changes: 0 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion core/src/snapshot_packager_service.rs
Original file line number Diff line number Diff line change
@@ -264,7 +264,7 @@ mod tests {
let bank_snapshot_info =
snapshot_utils::get_highest_bank_snapshot(&bank_snapshots_dir).unwrap();
let snapshot_storages = bank.get_snapshot_storages(None);
let archive_format = ArchiveFormat::TarBzip2;
let archive_format = ArchiveFormat::Tar;

let full_archive = snapshot_bank_utils::package_and_archive_full_snapshot(
&bank,
8 changes: 1 addition & 7 deletions download-utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -283,13 +283,7 @@ pub fn download_snapshot_archive(
});
fs::create_dir_all(&snapshot_archives_remote_dir).unwrap();

for archive_format in [
ArchiveFormat::TarZstd,
ArchiveFormat::TarGzip,
ArchiveFormat::TarBzip2,
ArchiveFormat::TarLz4,
ArchiveFormat::Tar, // `solana-test-validator` creates uncompressed snapshots
] {
for archive_format in [ArchiveFormat::TarZstd, ArchiveFormat::TarLz4] {
let destination_path = match snapshot_kind {
SnapshotKind::FullSnapshot => snapshot_utils::build_full_snapshot_archive_path(
&snapshot_archives_remote_dir,
2 changes: 0 additions & 2 deletions programs/sbf/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 0 additions & 2 deletions runtime/Cargo.toml
Original file line number Diff line number Diff line change
@@ -17,11 +17,9 @@ blake3 = { workspace = true }
bv = { workspace = true, features = ["serde"] }
bytemuck = { workspace = true }
byteorder = { workspace = true }
bzip2 = { workspace = true }
crossbeam-channel = { workspace = true }
dashmap = { workspace = true, features = ["rayon", "raw-api"] }
dir-diff = { workspace = true }
flate2 = { workspace = true }
fnv = { workspace = true }
fs-err = { workspace = true }
im = { workspace = true, features = ["rayon", "serde"] }
2 changes: 1 addition & 1 deletion runtime/src/bank/serde_snapshot.rs
Original file line number Diff line number Diff line change
@@ -474,7 +474,7 @@ mod tests {
None,
full_snapshot_archives_dir.path(),
incremental_snapshot_archives_dir.path(),
ArchiveFormat::TarBzip2,
ArchiveFormat::Tar,
NonZeroUsize::new(1).unwrap(),
NonZeroUsize::new(1).unwrap(),
)
2 changes: 1 addition & 1 deletion runtime/src/snapshot_bank_utils.rs
Original file line number Diff line number Diff line change
@@ -1408,7 +1408,7 @@ mod tests {
let bank_snapshots_dir = tempfile::TempDir::new().unwrap();
let full_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
let incremental_snapshot_archives_dir = tempfile::TempDir::new().unwrap();
let snapshot_archive_format = ArchiveFormat::TarGzip;
let snapshot_archive_format = ArchiveFormat::Tar;

let full_snapshot_archive_info = bank_to_full_snapshot_archive(
bank_snapshots_dir.path(),
42 changes: 3 additions & 39 deletions runtime/src/snapshot_utils.rs
Original file line number Diff line number Diff line change
@@ -10,9 +10,7 @@ use {
RebuiltSnapshotStorage, SnapshotStorageRebuilder,
},
},
bzip2::bufread::BzDecoder,
crossbeam_channel::Sender,
flate2::read::GzDecoder,
fs_err,
lazy_static::lazy_static,
log::*,
@@ -70,8 +68,9 @@ pub const DEFAULT_MAX_FULL_SNAPSHOT_ARCHIVES_TO_RETAIN: NonZeroUsize =
unsafe { NonZeroUsize::new_unchecked(2) };
pub const DEFAULT_MAX_INCREMENTAL_SNAPSHOT_ARCHIVES_TO_RETAIN: NonZeroUsize =
unsafe { NonZeroUsize::new_unchecked(4) };
pub const FULL_SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str = r"^snapshot-(?P<slot>[[:digit:]]+)-(?P<hash>[[:alnum:]]+)\.(?P<ext>tar|tar\.bz2|tar\.zst|tar\.gz|tar\.lz4)$";
pub const INCREMENTAL_SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str = r"^incremental-snapshot-(?P<base>[[:digit:]]+)-(?P<slot>[[:digit:]]+)-(?P<hash>[[:alnum:]]+)\.(?P<ext>tar|tar\.bz2|tar\.zst|tar\.gz|tar\.lz4)$";
pub const FULL_SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str =
r"^snapshot-(?P<slot>[[:digit:]]+)-(?P<hash>[[:alnum:]]+)\.(?P<ext>tar|tar\.zst|tar\.lz4)$";
pub const INCREMENTAL_SNAPSHOT_ARCHIVE_FILENAME_REGEX: &str = r"^incremental-snapshot-(?P<base>[[:digit:]]+)-(?P<slot>[[:digit:]]+)-(?P<hash>[[:alnum:]]+)\.(?P<ext>tar|tar\.zst|tar\.lz4)$";

#[derive(Copy, Clone, Default, Eq, PartialEq, Debug)]
pub enum SnapshotVersion {
@@ -783,18 +782,6 @@ pub fn archive_snapshot_package(
};

match snapshot_package.archive_format() {
ArchiveFormat::TarBzip2 => {
let mut encoder =
bzip2::write::BzEncoder::new(archive_file, bzip2::Compression::best());
do_archive_files(&mut encoder)?;
encoder.finish()?;
}
ArchiveFormat::TarGzip => {
let mut encoder =
flate2::write::GzEncoder::new(archive_file, flate2::Compression::default());
do_archive_files(&mut encoder)?;
encoder.finish()?;
}
ArchiveFormat::TarZstd => {
let mut encoder = zstd::stream::Encoder::new(archive_file, 0)?;
do_archive_files(&mut encoder)?;
@@ -1917,8 +1904,6 @@ fn untar_snapshot_create_shared_buffer(
) -> SharedBuffer {
let open_file = || fs_err::File::open(snapshot_tar).unwrap();
match archive_format {
ArchiveFormat::TarBzip2 => SharedBuffer::new(BzDecoder::new(BufReader::new(open_file()))),
ArchiveFormat::TarGzip => SharedBuffer::new(GzDecoder::new(BufReader::new(open_file()))),
ArchiveFormat::TarZstd => SharedBuffer::new(
zstd::stream::read::Decoder::new(BufReader::new(open_file())).unwrap(),
),
@@ -2339,14 +2324,6 @@ mod tests {

#[test]
fn test_parse_full_snapshot_archive_filename() {
assert_eq!(
parse_full_snapshot_archive_filename(&format!(
"snapshot-42-{}.tar.bz2",
Hash::default()
))
.unwrap(),
(42, SnapshotHash(Hash::default()), ArchiveFormat::TarBzip2)
);
assert_eq!(
parse_full_snapshot_archive_filename(&format!(
"snapshot-43-{}.tar.zst",
@@ -2411,19 +2388,6 @@ mod tests {

#[test]
fn test_parse_incremental_snapshot_archive_filename() {
assert_eq!(
parse_incremental_snapshot_archive_filename(&format!(
"incremental-snapshot-42-123-{}.tar.bz2",
Hash::default()
))
.unwrap(),
(
42,
123,
SnapshotHash(Hash::default()),
ArchiveFormat::TarBzip2
)
);
assert_eq!(
parse_incremental_snapshot_archive_filename(&format!(
"incremental-snapshot-43-234-{}.tar.zst",
41 changes: 3 additions & 38 deletions runtime/src/snapshot_utils/archive_format.rs
Original file line number Diff line number Diff line change
@@ -3,20 +3,17 @@ use {
strum::Display,
};

pub const SUPPORTED_ARCHIVE_COMPRESSION: &[&str] = &["bz2", "gzip", "zstd", "lz4", "tar", "none"];
// Publicly support "zstd" and "lz4", but retain support for "tar" for unit tests
pub const SUPPORTED_ARCHIVE_COMPRESSION: &[&str] = &["zstd", "lz4"];
pub const DEFAULT_ARCHIVE_COMPRESSION: &str = "zstd";

pub const TAR_BZIP2_EXTENSION: &str = "tar.bz2";
pub const TAR_GZIP_EXTENSION: &str = "tar.gz";
pub const TAR_ZSTD_EXTENSION: &str = "tar.zst";
pub const TAR_LZ4_EXTENSION: &str = "tar.lz4";
pub const TAR_EXTENSION: &str = "tar";

/// The different archive formats used for snapshots
#[derive(Copy, Clone, Debug, Eq, PartialEq, Display)]
pub enum ArchiveFormat {
TarBzip2,
TarGzip,
TarZstd,
TarLz4,
Tar,
@@ -26,8 +23,6 @@ impl ArchiveFormat {
/// Get the file extension for the ArchiveFormat
pub fn extension(&self) -> &str {
match self {
ArchiveFormat::TarBzip2 => TAR_BZIP2_EXTENSION,
ArchiveFormat::TarGzip => TAR_GZIP_EXTENSION,
ArchiveFormat::TarZstd => TAR_ZSTD_EXTENSION,
ArchiveFormat::TarLz4 => TAR_LZ4_EXTENSION,
ArchiveFormat::Tar => TAR_EXTENSION,
@@ -36,11 +31,8 @@ impl ArchiveFormat {

pub fn from_cli_arg(archive_format_str: &str) -> Option<ArchiveFormat> {
match archive_format_str {
"bz2" => Some(ArchiveFormat::TarBzip2),
"gzip" => Some(ArchiveFormat::TarGzip),
"zstd" => Some(ArchiveFormat::TarZstd),
"lz4" => Some(ArchiveFormat::TarLz4),
"tar" | "none" => Some(ArchiveFormat::Tar),
_ => None,
}
}
@@ -53,8 +45,6 @@ impl TryFrom<&str> for ArchiveFormat {

fn try_from(extension: &str) -> Result<Self, Self::Error> {
match extension {
TAR_BZIP2_EXTENSION => Ok(ArchiveFormat::TarBzip2),
TAR_GZIP_EXTENSION => Ok(ArchiveFormat::TarGzip),
TAR_ZSTD_EXTENSION => Ok(ArchiveFormat::TarZstd),
TAR_LZ4_EXTENSION => Ok(ArchiveFormat::TarLz4),
TAR_EXTENSION => Ok(ArchiveFormat::Tar),
@@ -93,23 +83,13 @@ mod tests {

#[test]
fn test_extension() {
assert_eq!(ArchiveFormat::TarBzip2.extension(), TAR_BZIP2_EXTENSION);
assert_eq!(ArchiveFormat::TarGzip.extension(), TAR_GZIP_EXTENSION);
assert_eq!(ArchiveFormat::TarZstd.extension(), TAR_ZSTD_EXTENSION);
assert_eq!(ArchiveFormat::TarLz4.extension(), TAR_LZ4_EXTENSION);
assert_eq!(ArchiveFormat::Tar.extension(), TAR_EXTENSION);
}

#[test]
fn test_try_from() {
assert_eq!(
ArchiveFormat::try_from(TAR_BZIP2_EXTENSION),
Ok(ArchiveFormat::TarBzip2)
);
assert_eq!(
ArchiveFormat::try_from(TAR_GZIP_EXTENSION),
Ok(ArchiveFormat::TarGzip)
);
assert_eq!(
ArchiveFormat::try_from(TAR_ZSTD_EXTENSION),
Ok(ArchiveFormat::TarZstd)
@@ -130,14 +110,6 @@ mod tests {

#[test]
fn test_from_str() {
assert_eq!(
ArchiveFormat::from_str(TAR_BZIP2_EXTENSION),
Ok(ArchiveFormat::TarBzip2)
);
assert_eq!(
ArchiveFormat::from_str(TAR_GZIP_EXTENSION),
Ok(ArchiveFormat::TarGzip)
);
assert_eq!(
ArchiveFormat::from_str(TAR_ZSTD_EXTENSION),
Ok(ArchiveFormat::TarZstd)
@@ -158,14 +130,7 @@ mod tests {

#[test]
fn test_from_cli_arg() {
let golden = [
Some(ArchiveFormat::TarBzip2),
Some(ArchiveFormat::TarGzip),
Some(ArchiveFormat::TarZstd),
Some(ArchiveFormat::TarLz4),
Some(ArchiveFormat::Tar),
Some(ArchiveFormat::Tar),
];
let golden = [Some(ArchiveFormat::TarZstd), Some(ArchiveFormat::TarLz4)];

for (arg, expected) in zip(SUPPORTED_ARCHIVE_COMPRESSION.iter(), golden.into_iter()) {
assert_eq!(ArchiveFormat::from_cli_arg(arg), expected);

0 comments on commit bd3d1fa

Please sign in to comment.