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

Add a simple timer support to schedule work at fixed times/intervals #6543

Closed
wants to merge 9 commits into from

Conversation

sagar0
Copy link
Contributor

@sagar0 sagar0 commented Mar 16, 2020

Adding a simple timer support to schedule work at a fixed time.

Test Plan:
Added new unit tests.

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.

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

Copy link
Contributor

@siying siying left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! It looks great! I left some comments on test code.

util/timer_test.cc Outdated Show resolved Hide resolved
util/timer_test.cc Outdated Show resolved Hide resolved
util/timer_test.cc Outdated Show resolved Hide resolved
util/timer_test.cc Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

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.

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

@sagar0 sagar0 removed the WIP Work in progress label Mar 27, 2020
@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

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.

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

@sagar0
Copy link
Contributor Author

sagar0 commented Mar 27, 2020

Somehow travis builds are no longer getting queued for this PR; not sure why.

@pdillinger
Copy link
Contributor

Somehow travis builds are no longer getting queued for this PR; not sure why.

I think just the link failed to be provided. https://travis-ci.org/github/facebook/rocksdb/builds/667842142

@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

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.

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

util/timer_test.cc Outdated Show resolved Hide resolved
@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

@sagar0
Copy link
Contributor Author

sagar0 commented Apr 6, 2020

Rebased to fix conflicts.

@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

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.

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

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.

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

@facebook-github-bot
Copy link
Contributor

@sagar0 has updated the pull request. Re-import the pull request

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.

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

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.

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

@sagar0
Copy link
Contributor Author

sagar0 commented Apr 7, 2020

All builds and tests passed. I don't know why AppVeyor is reporting a build failure when I see that all tests passed.
I am somehow unable to land this PR; @siying could you please help me land this?

@sagar0
Copy link
Contributor Author

sagar0 commented Apr 7, 2020

@siying
Copy link
Contributor

siying commented Apr 7, 2020

@sagar0 can you retry again, or press "Ship It" in https://our.internmc.facebook.com/intern/diff/D20465390/ ?

We have a new workflow now which requires accepting internally too. Now I accepted there and hope it works now.

@facebook-github-bot
Copy link
Contributor

@sagar0 merged this pull request in 0355d14.

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.

4 participants