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

Optimise deleteByTypeAndClient method #43

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

All notable changes to this project will be documented in this file, in reverse chronological order by release.

## 1.8.0 - 2022-04-29
- Add index definition for `deleteByTypeAndClient`
- `deleteByTypeAndClient` is now more performant without sorting in the query

## 1.7.0 - 2022-04-19
- `deleteOlderThan` method on `ActionRepository` now forces use of a primitive for batch size.
- Begin returning the number of rows deleted from deletion methods in `ActionRepository`
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
group=com.transferwise.idempotence4j
version=1.7.0
version=1.8.0
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ public int[] deleteByIds(List<ActionId> actionIdList) {
"DELETE FROM idempotent_action " +
"WHERE type = :type " +
" AND client = :client " +
"ORDER BY created_at ASC " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a good spot - it's not necessary for this to be deterministic!

"LIMIT :limit";

private final static String DELETE_BY_ACTION_ID_SQL =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CREATE INDEX IF NOT EXISTS IDX_IDEMPOTENT_ACTION_TYPE_CLIENT
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just googling to see if there's a mariadb equivalent of concurrently - looks like adding ALGORITHM=INPLACE LOCK=NONE may do the trick?

ON idempotent_action (type, client);
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,11 @@ class JdbcMariaDbActionRepositoryIntegrationTest extends IntegrationTest {
}
and:
List<ActionId> actionIds = actions.each { it -> repository.insertOrGet(it) } .collect({ it.actionId })
def firstHalfActionIds = actionIds.dropRight(actions.size() / 2 as int)
def lastHalfActionIds = actionIds.drop(actions.size() / 2 as int)
when:
int firstDeletionCount = repository.deleteByTypeAndClient(TYPE, CLIENT, 5)
then:
firstDeletionCount == 5
firstHalfActionIds.each { ActionId it -> assert repository.find(it).isEmpty() }
lastHalfActionIds.each { ActionId it -> assert !repository.find(it).isEmpty() }
actionIds.findAll { repository.find(it).isEmpty() }.size() == 5
when:
int secondDeletionCount = repository.deleteByTypeAndClient(TYPE, CLIENT, 5)
then:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ public int[] deleteByIds(List<ActionId> actionIds) {
"WHERE " +
"type = :type " +
"AND client = :client " +
"ORDER BY created_at ASC " +
"LIMIT :limit";

private final static String DELETE_BY_ACTION_ID_SQL =
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
CREATE INDEX CONCURRENTLY IF NOT EXISTS IDX_IDEMPOTENT_ACTION_TYPE_CLIENT
ON idempotent_action (type, client);
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,11 @@ class JdbcPostgresActionRepositoryIntegrationTest extends IntegrationTest {
}
and:
List<ActionId> actionIds = actions.each { it -> repository.insertOrGet(it) } .collect({ it.actionId })
def firstHalfActionIds = actionIds.dropRight(actions.size() / 2 as int)
def lastHalfActionIds = actionIds.drop(actions.size() / 2 as int)
when:
int firstDeletionCount = repository.deleteByTypeAndClient(TYPE, CLIENT, 5)
then:
firstDeletionCount == 5
firstHalfActionIds.each { ActionId it -> assert repository.find(it).isEmpty() }
lastHalfActionIds.each { ActionId it -> assert !repository.find(it).isEmpty() }
actionIds.findAll { repository.find(it).isEmpty() }.size() == 5
when:
int secondDeletionCount = repository.deleteByTypeAndClient(TYPE, CLIENT, 5)
then:
Expand Down