Skip to content

Conversation

mapleFU
Copy link
Member

@mapleFU mapleFU commented May 6, 2024

  1. Remove Get Column Family by name
  2. Add a "ListColumnFamily"

TBD: should we add a global CF manager with std::shared_ptr?

@mapleFU
Copy link
Member Author

mapleFU commented May 6, 2024

cc @jjz921024 @PragmaTwice @git-hulk

@mapleFU mapleFU marked this pull request as ready for review May 7, 2024 04:25
@@ -1334,11 +1334,19 @@ Status SlotMigrator::migrateIncrementalDataByRawKV(uint64_t end_seq, BatchSender
break;
}
case engine::WALItem::Type::kTypePut: {
if (item.column_family_id > kMaxColumnFamilyID) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This avoid get and put default CF

@mapleFU mapleFU requested a review from PragmaTwice May 11, 2024 14:27
@PragmaTwice
Copy link
Member

should we add a global CF manager with std::shared_ptr?

I don't think shared_ptr is useful here. Just put the ownership to the storage class.

Comment on lines 239 to 245
std::vector<std::string> cf_names_except_default;
const auto &all_column_family = ColumnFamilyConfigs::ListAllColumnFamily();
for (const auto &cf : all_column_family) {
if (cf.Name() != rocksdb::kDefaultColumnFamilyName) {
cf_names_except_default.emplace_back(cf.Name());
}
}
Copy link
Member

@PragmaTwice PragmaTwice May 11, 2024

Choose a reason for hiding this comment

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

all_column_family | transform([](..){ name }) | remove_if([](..){ v != default; })

You can use range-v3 here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Anyway the logic is just same here...

Copy link
Member

Choose a reason for hiding this comment

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

I'm thinking if we can move it into a function, like ColumnFamilyConfigs::ListColumnFamiliesWithoutDefault();

@mapleFU mapleFU force-pushed the refactor-cf-handling branch from ac845e5 to 7451082 Compare May 11, 2024 15:42
@mapleFU mapleFU force-pushed the refactor-cf-handling branch from 7451082 to 8869ace Compare May 11, 2024 15:51
@mapleFU mapleFU force-pushed the refactor-cf-handling branch from 8869ace to 0f332ae Compare May 11, 2024 15:53
@mapleFU mapleFU force-pushed the refactor-cf-handling branch from 5e976f6 to d0c9b19 Compare May 11, 2024 16:12
@mapleFU mapleFU force-pushed the refactor-cf-handling branch from d0c9b19 to dbf7d7b Compare May 11, 2024 16:14
Copy link

@mapleFU mapleFU requested review from git-hulk and PragmaTwice May 14, 2024 05:43
@mapleFU
Copy link
Member Author

mapleFU commented May 14, 2024

@git-hulk @PragmaTwice Would you mind take a look?

@git-hulk
Copy link
Member

@mapleFU Sure, will take a look today.

@PragmaTwice PragmaTwice merged commit 90e05f1 into apache:unstable May 14, 2024
@mapleFU mapleFU deleted the refactor-cf-handling branch May 14, 2024 12:07
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.

3 participants