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

ContextScheduler: multithread and timer problems #34

Open
ghost opened this issue Mar 2, 2015 · 4 comments
Open

ContextScheduler: multithread and timer problems #34

ghost opened this issue Mar 2, 2015 · 4 comments

Comments

@ghost
Copy link

ghost commented Mar 2, 2015

Encountered dubious flaky behaviour with some of the more complex wallclock-time observable ops like .debounce() and when using .subscribeOn() and .observeOn(). I traced the problems to ContextScheduler just not being quite right (IMHO)

  • you really have to save the vertx context and do your vertx ops on it, otherwise they tend to silently fail when the context is null. Stashing the context allows enqueueing of actions from other threads.
  • .schedule()/.schedulePeriodically() are supposed to return individually cancellable subscriptions that don't necessarily cancel the whole worker. Things like .debounce() apparently rely on the individual schedule subscription being cancellable without cancelling the entire worker.
  • vertx api deliberately rejects delays/periods < 1 msec, but rxjava really expects them to work
@squaredfinancialit
Copy link
Contributor

CLA signed for company (legal entity), submitting pull request #35

@petermd
Copy link
Contributor

petermd commented Mar 18, 2015

thanks for submitting this, and sorry for the delay with merging. did you create any tests to confirm the issues / resolution?

(if not i can look to add one)

@ghost
Copy link
Author

ghost commented Mar 18, 2015

...that would have been a reasonable thing to have done alright...

PR #35 has been updated with a few additions to SchedulerTest that fail on unpatched codebase. Beware, if trying them on the unpatched codebase, some will only eventually fail by hitting the global vertx.test.timeout (300 secs by default, adjustable by system property).

testConcurrentDebounce() may seem complicated, but one won't typically symptomatically encounter the issue with simple cases. The idea is to have an ongoing "slow" debounce and an ongoing "fast" debounce at once - without the patch the "fast" debounce detectably interferes with the "slow" debounce!

@ghost
Copy link
Author

ghost commented Mar 20, 2015

Coincidentally, turns out vertx 3 official vertx-rx has added some conceptually overlapping (though independently implemented) changes. Note how it has a TimedAction somewhat like (just in the sense it has a similar functional role, not any legal sense) the #35 ContextAction - i.e. instances of TimedAction are being returned from schedule()/schedulePeriodically()

(untested whether all the #34 sub-issues addressed here by #35 for vertx2 mod-rxvertx are thus already addressed in the vertx3 rx implementation (or were even present in that the first place), mind - e.g. just looking at the source code I don't see caching of the context at time of writing, but maybe in vertx3 things work differently)

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

No branches or pull requests

2 participants