-
Notifications
You must be signed in to change notification settings - Fork 655
New jcstress concurrency stress testing module #1054
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
base: master
Are you sure you want to change the base?
Conversation
|
Hi @the-thing , thanks for this, looks very useful. I also recalled that issue #357 when reading your description. |
|
I'm not sure about the CI integration. The issues I can see are
I don't think I have seen jcstress running as part of CI pipeline before. Normally when one suspects a concurrency bug. You write jcstress test, fix it and rerun. Having it as part of the project makes things easier with manual re-runs and setup. However, I have seen benchmarks being run automatically for continuous performance monitoring. It depends how you are planning on using it. I imagine there is another job that runs after the main CI build that always succeeds and if anyone is interested can view the results on demand. Is that what you are after? |
|
The main interest for me (besides finding existing bugs) was that no further bugs are introduced. Just wanted to automate it because otherwise I'll probably forget to run it on a regular basis. =) |
|
This is doable, but it might require additional effort. You would also have to expect that the current action's build times to increase significantly. You will have to write tests first. I would expect that whoever opens a new PR with concurrency fix will back it up with jcstress test and mention to run it. |
Introduce jcstress module for concurrency stress testing.
Changes
quickfix.JdbcStoreto allow testingNotes
I wrote two tests intially. One for
quickfix.JdbcStoreand one forquickfix.MemoryStoreas a starter.Each test encapsulates only a MemoryStore implementation, but both tests assume that sequence increment is guarded by their respective reentrant lock (which I'm not entirely sure about that this happens everywhere in the code).
Stress test clearly shows that there are concurrency issues in
quickfix.JdbcStoreimplementation - #357. See commens in JdbcStoreStressTest.java. This errors are rare - < 0.01% under specific circumstances.I think there might be more issues related to reading sequences by different threads e.g. "QFJ Message Processor" thread reading stale sender sequences modified by application threads, but it is hard to confirm.
I hope that I got the mocks right for
quickfix.JdbcStoretest. Let me know if you see any value in this.