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

fix(hotspot): replace partition_resolver to ddl_client #641

Merged
merged 9 commits into from
Nov 12, 2020

Conversation

Smityz
Copy link
Contributor

@Smityz Smityz commented Nov 9, 2020

After detect_hotkey_request was removed from rrdb_types, partition_resolver needs to be replaced to send RPC correctly.

src/server/info_collector.cpp Show resolved Hide resolved
src/server/info_collector.cpp Show resolved Hide resolved
@@ -316,7 +316,8 @@ info_collector::get_hotspot_calculator(const std::string &app_name, const int pa
if (iter != _hotspot_calculator_store.end()) {
return iter->second;
}
auto calculator = std::make_shared<hotspot_partition_calculator>(app_name, partition_count);
auto calculator =
std::make_shared<hotspot_partition_calculator>(app_name, partition_count, _shell_context);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think ddl_client is necessary for hotspot_partition_calculator, you can create a new ddl client instance for calculator, not share the shell_context with info_collector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There will be a lot of hotspot_partition_calculator(s) in one database, I think it is better to share one instance

src/server/hotspot_partition_calculator.h Show resolved Hide resolved
src/server/hotspot_partition_calculator.cpp Outdated Show resolved Hide resolved
int partition_count;
std::vector<dsn::partition_configuration> partitions;
_shell_context->ddl_client->list_app(app_name, app_id, partition_count, partitions);
auto target_address = partitions[partition_index].primary;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that you don't validate the partition_index, will the caller ensure that the partition_index is valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it will ensure

@acelyc111 acelyc111 merged commit 22238e7 into apache:master Nov 12, 2020
@Smityz Smityz deleted the auto-hotkey-detect branch November 13, 2020 09:06
@neverchanje neverchanje mentioned this pull request Mar 1, 2021
acelyc111 pushed a commit that referenced this pull request Jun 23, 2022
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.

5 participants