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: Optimized code logic #1834

Merged
merged 1 commit into from
Jul 29, 2023
Merged

fix: Optimized code logic #1834

merged 1 commit into from
Jul 29, 2023

Conversation

Mixficsol
Copy link
Collaborator

fix :Optimized code logic

gettimeofday(&last_check_resume_time_, nullptr);
if (disk_use_ratio < min_check_resume_ratio || free_size < least_free_size) {
return;
}

std::map<std::string, uint64_t> background_errors;
std::shared_lock db_rwl(g_pika_server->dbs_rw_);
Copy link

Choose a reason for hiding this comment

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

In the code patch you provided, here's a brief review:

  1. The conditional statement in line 1536 seems redundant and can be simplified. Instead of checking disk_use_ratio twice, you can remove the first condition and keep only free_size > least_free_size in the if statement.

Updated code:

if (disk_use_ratio > min_check_resume_ratio && free_size > least_free_size) {
  1. Line 1539 contains an unnecessary check for disk_use_ratio and free_size inside the if statement. This check is already performed in the previous if condition and can be safely removed.

  2. The code snippet you provided doesn't show the rest of the function, so it's difficult to fully evaluate its correctness or identify potential bugs. Make sure the remaining code within the function is correct and properly handles the intended logic.

  3. Consider adding comments to clarify the purpose and functionality of the code, especially if it involves any complex or non-obvious operations.

Overall, the code patch appears fine with the suggested simplifications. However, please note that a comprehensive review requires examining the entire context and related code.

@Mixficsol Mixficsol changed the title Optimized code logic fix: Optimized code logic Jul 28, 2023
gettimeofday(&last_check_resume_time_, nullptr);
if (disk_use_ratio < min_check_resume_ratio || free_size < least_free_size) {
return;
}

std::map<std::string, uint64_t> background_errors;
std::shared_lock db_rwl(g_pika_server->dbs_rw_);
Copy link

Choose a reason for hiding this comment

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

Based on the provided code patch, here's a brief code review:

  1. The condition in the if statement has been changed to include an additional check: free_size > least_free_size.

    • Previously, it only checked disk_use_ratio > min_check_resume_ratio.
    • Now, both conditions need to be satisfied for the code execution to proceed.
  2. The redundant conditional check inside the if-statement has been removed:

    • The previous code checked disk_use_ratio < min_check_resume_ratio || free_size < least_free_size after setting last_check_resume_time_.
    • This check is no longer needed since the new condition in the if-statement already covers it.

Improvement suggestion:

  • Given that the code initializes background_errors and acquires a shared lock on dbs_rw_, it might be worth considering whether these operations are necessary in all cases. If they can be avoided when the conditions aren't met, it could potentially improve performance.

Bug risk analysis:

  • Without further context about the surrounding code or requirements, it's not possible to determine if there are any hidden bug risks.
  • Make sure to test the code thoroughly with various input scenarios and compare the results against the expected behavior.
  • Be especially careful with the variables min_check_resume_ratio and least_free_size to ensure their values are set correctly and aligned with your intended logic.

@Mixficsol Mixficsol merged commit 2511dda into OpenAtomFoundation:unstable Jul 29, 2023
@Mixficsol Mixficsol deleted the add_disk_test branch July 29, 2023 12:05
chejinge added a commit that referenced this pull request Jul 31, 2023
Co-authored-by: Mixficsol <838844609@qq.com>
chejinge added a commit that referenced this pull request Aug 1, 2023
* Optimized code logic (#1834)

* fix:delete consensus level (#1844)

* delete_consensus_level

---------

Co-authored-by: Mixficsol <838844609@qq.com>
Mixficsol added a commit that referenced this pull request Aug 4, 2023
* Optimized code logic (#1834)

* fix:delete consensus level (#1844)

* delete_consensus_level

* Update pika.conf (#1854)

* fix:delete_consensus_level (#1852)

* delete_consensus_level

* fix:shutdown&&slaveof no one too slow (#1859)

* fix_slow_shut_down

* add go integrate test (#1792)

* add go integrate test

* add go integrate test

* add go integrate test

* add go integrate test

* temp test

* temp test

* temp test

* temp test

* temp test

* temp test

* add test

* temp test

* temp test

* add tcl

* add tcl

* reformat code

* add tcl

* reformat code

* add tcl

* reformat code

* add tcl

* fix setxx

* fix setxx

* add tcl

* add tcl

* fix: fix some compile error (#1861)

Co-authored-by: wangshaoyi <wangshaoyi@meituan.com>

* change-version

* feat: add diskrecovery command (#1843)

* Add diskrecovery command

* Add diskrecovery command

---------

Co-authored-by: chejinge <945997690@qq.com>
Co-authored-by: Yuecai Liu <38887641+luky116@users.noreply.github.com>
Co-authored-by: wangshao1 <30471730+wangshao1@users.noreply.github.com>
Co-authored-by: wangshaoyi <wangshaoyi@meituan.com>
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
cheniujh pushed a commit to cheniujh/pika that referenced this pull request Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants