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

Provide minSequenceNr for snapshot deletion #25590

Merged
merged 2 commits into from Sep 25, 2018

Conversation

Projects
None yet
4 participants
@chbatey
Member

chbatey commented Sep 10, 2018

Journals can use this to make the bulk deletion more efficient

Use keepNrBatches to delete the last few snapshots in case previous
deletes failed.

I started writing a test with a mock snapshot store but seemed overkill.

@akka-ci akka-ci added the validating label Sep 10, 2018

Show outdated Hide outdated akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala Outdated
Show outdated Hide outdated akka-cluster-sharding/src/main/scala/akka/cluster/sharding/Shard.scala Outdated
persistent actors should use the `deleteSnapshots` method.
persistent actors should use the `deleteSnapshots` method. Depending on the journal used this might be inefficient. It is
best practice to do specific deletes with `deleteSnapshot` or to include a `minSequenceNr` as well as a `maxSequenceNr`
for the `SnapshotSelectionCriteria`.

This comment has been minimized.

@patriknw

patriknw Sep 10, 2018

Member

good

@patriknw

patriknw Sep 10, 2018

Member

good

@akka-ci akka-ci added tested and removed validating labels Sep 10, 2018

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 10, 2018

Collaborator

Test PASSed.

Collaborator

akka-ci commented Sep 10, 2018

Test PASSed.

@akka-ci akka-ci added validating and removed tested labels Sep 10, 2018

@patriknw

LGTM

@akka-ci akka-ci added tested and removed validating labels Sep 10, 2018

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 10, 2018

Collaborator

Test PASSed.

Collaborator

akka-ci commented Sep 10, 2018

Test PASSed.

@chbatey chbatey added this to the 2.5.17 milestone Sep 14, 2018

@chbatey

This comment has been minimized.

Show comment
Hide comment
@chbatey

chbatey Sep 14, 2018

Member

Set the milestone as promised it would be in the next release

Member

chbatey commented Sep 14, 2018

Set the milestone as promised it would be in the next release

@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 18, 2018

Collaborator

Test FAILed.

Collaborator

akka-ci commented Sep 18, 2018

Test FAILed.

chbatey added some commits Sep 10, 2018

Provide minSequenceNr for snapshot deletion
Journals can use this to make the bulk deletion more efficient

Use keepNrBatches to delete the last few snapshots in case previous
deletes failed.
@akka-ci

This comment has been minimized.

Show comment
Hide comment
@akka-ci

akka-ci Sep 20, 2018

Collaborator

Test PASSed.

Collaborator

akka-ci commented Sep 20, 2018

Test PASSed.

@johanandren

LGTM (if i understood the range part correctly)

log.debug("PersistentShard messages to {} deleted successfully", toSequenceNr)
deleteSnapshots(SnapshotSelectionCriteria(maxSequenceNr = toSequenceNr - 1))
val deleteTo = toSequenceNr - 1
val deleteFrom = math.max(0, deleteTo - (keepNrOfBatches * snapshotAfter))

This comment has been minimized.

@johanandren

johanandren Sep 21, 2018

Member

So the low end of the range is a performance opt to not always delete from 0?

@johanandren

johanandren Sep 21, 2018

Member

So the low end of the range is a performance opt to not always delete from 0?

This comment has been minimized.

@patriknw

patriknw Sep 21, 2018

Member

that's right, and it's used in Cassandra plugin when akka/akka-persistence-cassandra#394 has been released

@patriknw

patriknw Sep 21, 2018

Member

that's right, and it's used in Cassandra plugin when akka/akka-persistence-cassandra#394 has been released

@chbatey chbatey merged commit 23b9266 into akka:master Sep 25, 2018

3 checks passed

Jenkins PR Validation Test PASSed. 704 tests run, 7 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
typesafe-cla-validator All users have signed the CLA
Details

@chbatey chbatey deleted the chbatey:sharding-snaphost-deletion branch Sep 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment