Repository navigation
Improve performance of deletes over many partitions, #384 - #385
Conversation
|
You also don't need to query |
There was a problem hiding this comment.
should be highestDeletedSequenceNumber + 1
There was a problem hiding this comment.
I propose to still flatMap physicalDelete future, for instance we have covered all journal methods with metrics and did not see that deletion takes so much time.
There was a problem hiding this comment.
I think the reason was that the physical delete could run in the background without affecting the response time of the PersistentActor.
On the other hand I guess there is no reason for a PersistentActor to "wait" or depend on how long time it takes for the delete to complete, so one could say that the entire delete is running in the background.
One problem with adding that to the response time might be that it could trigger the CircuitBreaker in Akka.
Let's see if there are other opinions when this PR is reviewed.
There was a problem hiding this comment.
This makes sense so that if a user actor gets a delete success they are definitely fully deleted.
Also completing the Future earlier only effects when the user actor gets the delete success it doesn't block further writes so it isn't really optimising anything unless the user is doing a side effect on delete success.
Maybe under a diff PR or commit?
There was a problem hiding this comment.
sure, I'll change that tomorrow morning
Fixed, thanks |
There was a problem hiding this comment.
This makes sense so that if a user actor gets a delete success they are definitely fully deleted.
Also completing the Future earlier only effects when the user actor gets the delete success it doesn't block further writes so it isn't really optimising anything unless the user is doing a side effect on delete success.
Maybe under a diff PR or commit?
* The lowest sequence number is always the stored highestDeletedSequenceNumber, instead of 1 * Use the given toSequenceNumber as highest instead of searching for highest, but Long.MaxValue can be used for deleting all events without knowing the highest * The main problem was that it was searching for highest sequence number in all partitions. * no need to query PartitionInfo
2e088aa to
3443a34
Compare
| highestSequenceNr <- asyncFindHighestSequenceNr(persistenceId, lowestSequenceNr, config.targetPartitionSize) | ||
| _ <- logicalDeleteWithAsyncPhysicalDelete(lowestSequenceNr, highestSequenceNr) | ||
| highestSequenceNr <- { | ||
| // MaxValue may be used as magic value to delete all events without specifying actual toSequenceNr |
There was a problem hiding this comment.
I have also added a check of valid seqnr in Akka akka/akka-core#25488
|
Looks good, one question about when to kick off the next delete if they are queued |
* The reason was that the physical delete could run in the background without affecting the response time of the PersistentActor. * However, there is no reason for a PersistentActor to "wait" or depend on how long time it takes for the delete to complete, so one could say that the entire delete is running in the background. * This change will make it look like deletes take longer time and may in worst case trigger CircuitBreaker in Akka, but it was considered better to make that visible than silently running parts of it in the background.
3443a34 to
6304fd5
Compare
|
pushed small rename |
3 participants
instead of 1
but Long.MaxValue can be used for deleting all events without knowing the
highest
all partitions.
Refs #384