Skip to content

Provided a better solution for reading dead outbox entries #1940

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

t-bonk
Copy link
Contributor

@t-bonk t-bonk commented Jun 27, 2025

No description provided.

@t-bonk t-bonk requested a review from BraunMatthias June 27, 2025 12:49
@t-bonk t-bonk requested a review from smahati as a code owner June 27, 2025 12:49
}

private Optional<Predicate> createOutboxFilters(CdsRuntime runtime) {
List<OutboxService> outboxServices = runtime.getServiceCatalog().getServices(OutboxService.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

what about iterating the persistent outboxes from application configuration (outboxConfigs)+ the known defaults? INMEMORY_NAME would not be required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BraunMatthias If an outbox has the default configuration, iterating over the configurations doesn't return the specific config for an outbox. I observed this while testing the code; in this case no outbox can be found and hence no dead entries are read at all.

Copy link
Contributor

Choose a reason for hiding this comment

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

This might be only true for the two default outboxes which needed to be explicitly named here. But what happens in 398 for those?

Predicate where = null;
for(OutboxService service : outboxServices) {
OutboxServiceConfig config = outboxConfigs.getService(service.getName());
Predicate targetPredicate = CQL.get(Messages.TARGET).eq(service.getName()).and(CQL.get(Messages.ATTEMPTS).ge(config.getMaxAttempts()));
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to filter by individual outbox and then OR the condition meeting all outboxes?

Copy link
Contributor Author

@t-bonk t-bonk Jul 18, 2025

Choose a reason for hiding this comment

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

@BraunMatthias Every outbox can have its own configuration for maxAttempts. To read only the dead entries per outbox, this filter is required.

Copy link
Contributor

Choose a reason for hiding this comment

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

same reason - I just overlooked this aspect

@t-bonk t-bonk requested a review from BraunMatthias July 18, 2025 08:40
This filtering can't be done on the database since the maximum number of attempts is only available from the CDS properties.

To ensure that only dead outbox entries are returned when reading `DeadOutboxMessages`, the following code provides the handler for the `DeadLetterQueueService` and the `@After-READ` handler that filters for the dead outbox entries:
Filtering the dead entries is done by adding by adding an appropriate `where`-clause to `READ`-queries on the outbox message entries which reached maximum number of retries. The following code provides an example handler implementation defining this behaviour for the `DeadLetterQueueService`:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Filtering the dead entries is done by adding by adding an appropriate `where`-clause to `READ`-queries on the outbox message entries which reached maximum number of retries. The following code provides an example handler implementation defining this behaviour for the `DeadLetterQueueService`:
Filtering the dead entries is done by adding an appropriate `where`-clause to all `READ`-queries which covers all outbox message entries with maximum number of retries. The following code provides an example handler implementation defining this behaviour for the `DeadLetterQueueService`:

}

private Optional<Predicate> createOutboxFilters(CdsRuntime runtime) {
List<OutboxService> outboxServices = runtime.getServiceCatalog().getServices(OutboxService.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be only true for the two default outboxes which needed to be explicitly named here. But what happens in 398 for those?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants