Skip to content

Commit

Permalink
Merge 'sstables: change make_descriptor() to accept fs::path' from Ke…
Browse files Browse the repository at this point in the history
…fu Chai

to lower the programmer's cognitive load. as programmer might want
to pass the full path as the `fname` when calling
`make_descriptor(sstring sstdir, sstring fname)`, but this overload
only accepts the filename component as its second parameter. a
single `path` parameter would be easier to work with.

Refs scylladb#15187

Closes scylladb#15188

* github.com:scylladb/scylladb:
  sstable: add samples of fname to be matched by regex
  sstables: change make_descriptor() to accept fs::path
  sstables: switch entry_descriptor(sstring..) to std::string_view
  sstables: change make_descriptor() to accept fs::path
  • Loading branch information
denesb committed Sep 1, 2023
2 parents a4eb3c6 + 94a056b commit b698666
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 17 deletions.
2 changes: 1 addition & 1 deletion replica/table.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,7 @@ future<std::unordered_map<sstring, table::snapshot_details>> table::get_snapshot
// All the others should just generate an exception: there is something wrong, so don't blindly
// add it to the size.
if (name != "manifest.json" && name != "schema.cql") {
sstables::entry_descriptor::make_descriptor(snapshot_dir.native(), name);
sstables::entry_descriptor::make_descriptor(snapshot_dir / name);
all_snapshots.at(snapshot_name).total += size;
} else {
size = 0;
Expand Down
6 changes: 3 additions & 3 deletions sstables/open_info.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,13 @@ struct entry_descriptor {
sstable_format_types format;
component_type component;

static entry_descriptor make_descriptor(sstring sstdir, sstring fname);
static entry_descriptor make_descriptor(const std::filesystem::path& sst_path);

// Use the given ks and cf and don't attempt to extract it from the dir path.
// This allows loading sstables from any path, but the filename still has to be valid.
static entry_descriptor make_descriptor(sstring sstdir, sstring fname, sstring ks, sstring cf);
static entry_descriptor make_descriptor(const std::filesystem::path& sst_path, sstring ks, sstring cf);

entry_descriptor(sstring sstdir, sstring ks, sstring cf, generation_type generation,
entry_descriptor(std::string_view sstdir, sstring ks, sstring cf, generation_type generation,
sstable_version_types version, sstable_format_types format,
component_type component)
: sstdir(sstdir), ks(ks), cf(cf), generation(generation), version(version), format(format), component(component) {}
Expand Down
7 changes: 4 additions & 3 deletions sstables/sstable_directory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,9 @@ future<> sstable_directory::filesystem_components_lister::process(sstable_direct
if (!de) {
break;
}
auto comps = sstables::entry_descriptor::make_descriptor(_directory.native(), de->name);
handle(std::move(comps), _directory / de->name);
auto component_path = _directory / de->name;
auto comps = sstables::entry_descriptor::make_descriptor(component_path);
handle(std::move(comps), component_path);
}
} catch (...) {
ex = std::current_exception();
Expand Down Expand Up @@ -405,7 +406,7 @@ sstable_directory::collect_output_unshared_sstables(std::vector<sstables::shared
}

dirlog.trace("Collected output SSTable {} is remote. Storing it", sst->get_filename());
_unshared_remote_sstables[shard].push_back(sstables::entry_descriptor::make_descriptor(_sstable_dir.native(), sst->component_basename(component_type::Data)));
_unshared_remote_sstables[shard].push_back(sstables::entry_descriptor::make_descriptor(_sstable_dir / sst->component_basename(component_type::Data)));
return make_ready_future<>();
});
}
Expand Down
14 changes: 9 additions & 5 deletions sstables/sstables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2101,7 +2101,11 @@ sstable::make_crawling_reader(
return kl::make_crawling_reader(shared_from_this(), std::move(schema), std::move(permit), std::move(trace_state), monitor);
}

static entry_descriptor make_entry_descriptor(sstring sstdir, sstring fname, sstring* const provided_ks, sstring* const provided_cf) {
static entry_descriptor make_entry_descriptor(std::string_view sstdir, std::string_view fname, sstring* const provided_ks, sstring* const provided_cf) {
// examples of fname look like
// la-42-big-Data.db
// ka-42-big-Data.db
// me-3g8w_00qf_4pbog2i7h2c7am0uoe-big-Data.db
static boost::regex la_mx("(la|m[cde])-([^-]+)-(\\w+)-(.*)");
static boost::regex ka("(\\w+)-(\\w+)-ka-(\\d+)-(.*)");

Expand Down Expand Up @@ -2156,12 +2160,12 @@ static entry_descriptor make_entry_descriptor(sstring sstdir, sstring fname, sst
return entry_descriptor(sstdir, ks, cf, generation_type::from_string(generation), version, format_from_string(format), sstable::component_from_sstring(version, component));
}

entry_descriptor entry_descriptor::make_descriptor(sstring sstdir, sstring fname) {
return make_entry_descriptor(std::move(sstdir), std::move(fname), nullptr, nullptr);
entry_descriptor entry_descriptor::make_descriptor(const std::filesystem::path& sst_path) {
return make_entry_descriptor(sst_path.parent_path().native(), sst_path.filename().native(), nullptr, nullptr);
}

entry_descriptor entry_descriptor::make_descriptor(sstring sstdir, sstring fname, sstring ks, sstring cf) {
return make_entry_descriptor(std::move(sstdir), std::move(fname), &ks, &cf);
entry_descriptor entry_descriptor::make_descriptor(const std::filesystem::path& sst_path, sstring ks, sstring cf) {
return make_entry_descriptor(sst_path.parent_path().native(), sst_path.filename().native(), &ks, &cf);
}

sstable_version_types version_from_string(std::string_view s) {
Expand Down
8 changes: 3 additions & 5 deletions tools/scylla-sstable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,8 @@ schema_ptr try_load_schema_autodetect(const bpo::variables_map& app_config) {
if (app_config.count("sstables")) {
try {
auto sst_path = std::filesystem::path(app_config["sstables"].as<std::vector<sstring>>().front());
auto ed = sstables::entry_descriptor::make_descriptor(sst_path);
const auto sst_dir_path = std::filesystem::path(sst_path).remove_filename();
const auto sst_filename = sst_path.filename();
auto ed = sstables::entry_descriptor::make_descriptor(sst_dir_path.native(), sst_filename.native());
std::filesystem::path data_dir_path;
// Detect whether sstable is in root table directory, or in a sub-directory
// The last component is "" due to the trailing "/" left by "remove_filename()" above.
Expand Down Expand Up @@ -253,10 +252,9 @@ const std::vector<sstables::shared_sstable> load_sstables(schema_ptr schema, sst
}
}

const auto dir_path = sst_path.parent_path();
const auto sst_filename = sst_path.filename();

auto ed = sstables::entry_descriptor::make_descriptor(dir_path.c_str(), sst_filename.c_str(), schema->ks_name(), schema->cf_name());
auto ed = sstables::entry_descriptor::make_descriptor(sst_path, schema->ks_name(), schema->cf_name());
const auto dir_path = sst_path.parent_path();
data_dictionary::storage_options local;
auto sst = sst_man.make_sstable(schema, dir_path.c_str(), local, ed.generation, sstables::sstable_state::normal, ed.version, ed.format);

Expand Down

0 comments on commit b698666

Please sign in to comment.