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

Document: calling persist() in Futures is bad #21689

Closed
eugenemiretsky opened this issue Oct 17, 2016 · 6 comments
Closed

Document: calling persist() in Futures is bad #21689

eugenemiretsky opened this issue Oct 17, 2016 · 6 comments
Assignees
Milestone

Comments

@eugenemiretsky
Copy link

When testing a persistent actor with a low throughput (one command at a time) event sometimes get "stuck" and are not processes until the next command is sent.

I suspect that it has to do with batching in EventSourced - events get batched before being sent to the persistent store. FlushBatch is getting called inside aroundReceiveComplete, which explains why the batch gets flushed when the 2nd command is sent.

Is there a way to tune, or turn off batching?

@ktoso
Copy link
Member

ktoso commented Oct 17, 2016

  • What persistence journal?
  • Are you using persist or persistAsync or a mix?
  • What does "low throughput" mean in your case?
  • Could you explain more about what you mean with "stuck"? By "command is sent" I assume you mean "event is stored", i.e. persist call happens?

@ktoso ktoso added the 0 - new Ticket is unclear on it's purpose or if it is valid or not label Oct 17, 2016
@eugenemiretsky
Copy link
Author

  1. LevelDB
  2. Persist
  3. low throughput - test load. Generating 1 request at a time manually.
  4. By stuck I mean that all the code before persist() is executed, however none of the code in the call back is executed. The callback is executed only after I send another command that triggers another persist call.

@patriknw
Copy link
Member

Sounds interesting. Do you have a reproducer?

@eugenemiretsky
Copy link
Author

Working on a reproducer.
Persist is getting called in a Future. Could that be the issue?

@patriknw
Copy link
Member

Yes, that is not allowed. Must be done in the actor's receive thread. Pipe the result of the future back to the actor and persist from receiveCommand

@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

Not a bug IMHO if persist() was called from a Future, incorrect usage though.
I added a more explicit warning about this: #21699

@ktoso ktoso added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:docs labels Oct 19, 2016
@ktoso ktoso self-assigned this Oct 19, 2016
@ktoso ktoso added this to the 2.4.12 milestone Oct 19, 2016
@ktoso ktoso added 3 - in progress Someone is working on this ticket and removed 0 - new Ticket is unclear on it's purpose or if it is valid or not 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted labels Oct 19, 2016
ktoso added a commit that referenced this issue Oct 21, 2016
* +doc,per #21689 document calling persist() in Future is unsafe

* Update persistence.rst

* Update persistence.rst
@ktoso ktoso closed this as completed Oct 21, 2016
@ktoso ktoso removed the 3 - in progress Someone is working on this ticket label Oct 21, 2016
@ktoso ktoso changed the title Events are 'stuck' in a persistent actor under low throughput Document: calling persist() in Futures is bad Oct 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants