Skip to content

Commit 5cd6e65

Browse files
committed
make the scheduled attachment cleanup multip pod save
1 parent 63ff658 commit 5cd6e65

4 files changed

Lines changed: 51 additions & 17 deletions

File tree

src/main/java/it/aboutbits/springboot/emailservice/lib/application/CleanupAttachmentFiles.java

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,24 @@ public class CleanupAttachmentFiles {
2929
void cleanupAttachments() {
3030
logStartOfPass();
3131

32-
var emailsToCleanup = queryEmail.readyToCleanup();
32+
var candidateIds = queryEmail.candidateIdsToCleanup();
3333

34+
var countClaimed = 0;
3435
var countCleaned = 0;
3536
var countError = 0;
36-
for (var email : emailsToCleanup) {
37+
for (var id : candidateIds) {
38+
var claimed = manageEmail.tryClaimForCleanup(id);
39+
40+
if (claimed.isEmpty()) {
41+
// Lost race to another pod; Skip
42+
continue;
43+
}
44+
countClaimed++;
45+
3746
try {
38-
manageEmail.cleanupAttachments(email);
47+
manageEmail.completeClaimedCleanup(claimed.get());
3948
countCleaned++;
40-
} catch (AttachmentException e) {
49+
} catch (AttachmentException _) {
4150
countError++;
4251
}
4352
}
@@ -46,7 +55,7 @@ void cleanupAttachments() {
4655

4756
for (var callback : callbacks) {
4857
callback.report(new AttachmentCleanerCallback.Report(
49-
emailsToCleanup.size(),
58+
countClaimed,
5059
countCleaned,
5160
countError
5261
));

src/main/java/it/aboutbits/springboot/emailservice/lib/application/ManageEmail.java

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ public ManageEmail(
6060
this.emailMapper = emailMapper;
6161
this.maxAttempts = maxAttempts;
6262
this.schedulerInterval = schedulerInterval;
63-
// Persist the final email state in its own, independent transaction for sendOrFail to never make it roll back
63+
// Persist the final email state in its own, independent transaction to never make it roll back
6464
this.transactionTemplate = new TransactionTemplate(transactionManager);
6565
this.transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW);
6666
}
@@ -142,12 +142,25 @@ Email completeClaimedSend(Email email) {
142142
return emailRepository.save(email);
143143
}
144144

145-
void cleanupAttachments(final Email email) throws AttachmentException {
146-
for (var attachment : email.getAttachments()) {
147-
attachmentDataSource.releaseAttachment(attachment.getFileReference());
145+
// Atomically flips a not-yet-cleaned SENT row to cleaned
146+
@Transactional
147+
Optional<Email> tryClaimForCleanup(long id) {
148+
var claimed = emailRepository.claimForCleanup(id);
149+
return claimed == 0 ? Optional.empty() : emailRepository.findById(id);
150+
}
151+
152+
// Actually release the attachment payloads of the claimed email
153+
void completeClaimedCleanup(final Email email) throws AttachmentException {
154+
try {
155+
for (var attachment : email.getAttachments()) {
156+
attachmentDataSource.releaseAttachment(attachment.getFileReference());
157+
}
158+
} catch (AttachmentException e) {
159+
// Undo the claim so the row is retried on a later pass
160+
email.setAttachmentsCleaned(false);
161+
transactionTemplate.execute(_ -> emailRepository.save(email));
162+
throw e;
148163
}
149-
email.setAttachmentsCleaned(true);
150-
emailRepository.save(email);
151164
}
152165

153166
private void sendMail(Email email) throws MessagingException, IOException, AttachmentException {

src/main/java/it/aboutbits/springboot/emailservice/lib/application/QueryEmail.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import it.aboutbits.springboot.emailservice.lib.EmailDto;
55
import it.aboutbits.springboot.emailservice.lib.EmailState;
66
import it.aboutbits.springboot.emailservice.lib.jpa.EmailRepository;
7-
import it.aboutbits.springboot.emailservice.lib.model.Email;
87
import lombok.RequiredArgsConstructor;
98
import org.jspecify.annotations.NullMarked;
109
import org.springframework.data.domain.Page;
@@ -47,8 +46,8 @@ List<Long> candidateIdsToSend(OffsetDateTime staleSendingBefore) {
4746
);
4847
}
4948

50-
List<Email> readyToCleanup() {
51-
return emailRepository.findReadyToCleanup();
49+
List<Long> candidateIdsToCleanup() {
50+
return emailRepository.findCandidateIdsToCleanup();
5251
}
5352

5453
public Optional<EmailDto> byId(long id) {

src/main/java/it/aboutbits/springboot/emailservice/lib/jpa/EmailRepository.java

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,24 @@ int claimForSend(
8484
@Param("staleSendingBefore") OffsetDateTime staleSendingBefore
8585
);
8686

87-
@EntityGraph(value = Email.DEFAULT_ENTITY_GRAPH)
87+
// Plain read, no locking -> two pods may see overlapping candidate sets.
88+
// The atomic UPDATE in claimForCleanup arbitrates the actual claim.
8889
@Query("""
89-
select e from Email e
90+
select e.id from Email e
9091
where e.attachmentsCleaned = false
9192
and e.state = it.aboutbits.springboot.emailservice.lib.EmailState.SENT
9293
""")
93-
List<Email> findReadyToCleanup();
94+
List<Long> findCandidateIdsToCleanup();
95+
96+
// Atomic compare-and-set claim: flips a single not-yet-cleaned SENT row to cleaned.
97+
// Concurrent updates are serialized against the same row, so EXACTLY ONE caller gets returned 1.
98+
@Modifying(clearAutomatically = true, flushAutomatically = true)
99+
@Query("""
100+
update Email e
101+
set e.attachmentsCleaned = true
102+
where e.id = :id
103+
and e.attachmentsCleaned = false
104+
and e.state = it.aboutbits.springboot.emailservice.lib.EmailState.SENT
105+
""")
106+
int claimForCleanup(@Param("id") long id);
94107
}

0 commit comments

Comments
 (0)