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

Shutdown timer in destructor #7292

Closed
wants to merge 2 commits into from
Closed

Conversation

jay-zhuang
Copy link
Contributor

Make sure deleting a running timer works fine.

Test Plan: unittest and an invalid benchmark command: ./db_bench --db=/tmp --use_existing_db=false --benchmarks=fred --compression_type=none

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@riversand963 riversand963 left a comment

Choose a reason for hiding this comment

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

Thanks @jay-zhuang for the fix.

[&] { mock_env_->set_current_time(mock_time_sec); });

// delete a running timer should not cause any exception
delete timer_ptr;
Copy link
Contributor

Choose a reason for hiding this comment

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

When deleting the timer, fn_sch_test may or may not be running, so maybe test two cases separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added another test for deleting a timer with running function (similar to shutdown running function test).

Make sure deleting a running timer is fine.

Test Plan: unittest and an invalid benchmark command: `./db_bench --db=/tmp --use_existing_db=false --benchmarks=fred --compression_type=none`
@facebook-github-bot
Copy link
Contributor

@jay-zhuang has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@jay-zhuang has updated the pull request. You must reimport the pull request before landing.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@jay-zhuang merged this pull request in e500c73.

codingrhythm pushed a commit to SafetyCulture/rocksdb that referenced this pull request Mar 5, 2021
Summary:
Make sure deleting a running timer works fine.

Pull Request resolved: facebook#7292

Test Plan: unittest and an invalid benchmark command: `./db_bench --db=/tmp --use_existing_db=false --benchmarks=fred --compression_type=none`

Reviewed By: riversand963

Differential Revision: D23248500

Pulled By: jay-zhuang

fbshipit-source-id: 04111681b389a9aa23a439db4568d5ca351f1144
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.

db_bench throws an exception when passed an invalid benchmark
3 participants