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

threadDelay & registerDelay - unbounded delays #2135

Merged
merged 2 commits into from
May 26, 2020
Merged

Conversation

coot
Copy link
Contributor

@coot coot commented May 22, 2020

This PR updates both threadDelay and registerDelay. Both accept
DiffTime, and they should be safe.

threadDelay is easy, since it's a synchronous operation;
registerDelay is more tricky as the returned TVar must be updated
asynchronously. For delays smaller than maxBound :: Int we reuse the original
STM implenetation, for larger delays we use the default 'MonadTimerimplementation (which starts a thread to update theTVar`).

  • registerDelay - accept unbounded delays
  • threadDaley - accept unbounded delays

@coot coot added networking io-sim Issues related to io-sim and io-sim-classes. labels May 22, 2020
@coot coot requested a review from dcoutts May 22, 2020 06:13
@coot
Copy link
Contributor Author

coot commented May 22, 2020

@mrBliss @kderme this is related to #2095

@coot coot requested a review from mrBliss May 22, 2020 06:17
@coot
Copy link
Contributor Author

coot commented May 22, 2020

@mrBliss there's a CI failure in Byron test suite: https://hydra.iohk.io/build/2717445/nixlog/3
Could this be caused by this PR?

@mrBliss
Copy link
Contributor

mrBliss commented May 22, 2020

@mrBliss there's a CI failure in Byron test suite: https://hydra.iohk.io/build/2717445/nixlog/3
Could this be caused by this PR?

If threadDelay no longer works correctly, then yes. But the implementations look fine and no other tests fail 🤔. Can you try to reproduce it without your changes?

@coot
Copy link
Contributor Author

coot commented May 22, 2020

Locally, it run fine:

    topology prevents timely dlg cert tx propagation:                                      OK (16.20s)

Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

LGTM!

We should also remove the duplicate implementation in the consensus code.

@mrBliss
Copy link
Contributor

mrBliss commented May 26, 2020

We should also remove the duplicate implementation in the consensus code.

Indeed. @kderme Can you take care of this? It'll boil down to reverting #2095.

coot added 2 commits May 26, 2020 18:34
For delays smaler than 'maxBound :: Int', we use
'Control.Monad.STM.registerDelay'; for larger delays we start
a monitoring thread which will update the returned 'TVar' (re-using the
default implementation).
@coot
Copy link
Contributor Author

coot commented May 26, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented May 26, 2020

@iohk-bors iohk-bors bot merged commit 0143c51 into master May 26, 2020
@iohk-bors iohk-bors bot deleted the coot/registerDelay branch May 26, 2020 17:34
@kderme kderme mentioned this pull request May 28, 2020
iohk-bors bot added a commit that referenced this pull request May 28, 2020
2162: Undo MonadDelay wrapper r=mrBliss a=kderme

After #2135 the wrapper is no longer needed.
Undoes #2095

Co-authored-by: kderme <k.dermenz@gmail.com>
coot added a commit that referenced this pull request May 16, 2022
2135: threadDelay & registerDelay - unbounded delays r=coot a=coot

This PR updates both `threadDelay` and `registerDelay`.  Both accept
`DiffTime`, and they should be safe.

`threadDelay` is easy, since it's a synchronous operation;
`registerDelay` is more tricky as the returned `TVar` must be updated
asynchronously. For delays smaller than `maxBound :: Int` we reuse the original
`STM` implenetation, for larger delays we use the default 'MonadTimer`
implementation (which starts a thread to update the `TVar`).

- registerDelay - accept unbounded delays
- threadDaley - accept unbounded delays


Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
coot pushed a commit that referenced this pull request May 16, 2022
2162: Undo MonadDelay wrapper r=mrBliss a=kderme

After #2135 the wrapper is no longer needed.
Undoes #2095

Co-authored-by: kderme <k.dermenz@gmail.com>
coot pushed a commit to input-output-hk/typed-protocols that referenced this pull request May 16, 2022
2162: Undo MonadDelay wrapper r=mrBliss a=kderme

After IntersectMBO/ouroboros-network#2135 the wrapper is no longer needed.
Undoes IntersectMBO/ouroboros-network#2095

Co-authored-by: kderme <k.dermenz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io-sim Issues related to io-sim and io-sim-classes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants