Consider improving message polling in JdbcMessageStore [INT-2200]

[spring-operator]

Oleg Zhurakousky opened INT-2200 and commented

Since 2.1.M3 'pollMessageFromGroup()' method was added to MessageGroupStore

Its current JDBC implementation while improved (see #6168) is not yet perfect since it still fetches all the rows from the table while only needing the first one.

Based on the PR comment form Gary https://github.com/SpringSource/spring-integration/pull/152/files#r185163

We should consider using JdbcPagingItemReader

---

Affects: 2.1 M2

This issue is a sub-task of #6523

Issue Links:

• Add locking strategy to AbstractMessageGroupStore [INT-1117] #5123 Add locking strategy to AbstractMessageGroupStore

• Support rescheduling of release tasks for a persistent MessageStore-backed DelayHandler upon restart [INT-1132] #5139 Support rescheduling of release tasks for a persistent MessageStore-backed DelayHandler upon restart

[spring-operator]

Oleg Zhurakousky commented

Moving it to 2.2.M1 since we need to come up with the strategy on how to depend on JdbcPagingItemReader without depending on Spring Batch. The actual class and its supporting infrastructure might need to move to some commons package either in Spring or spring-data-commons

[spring-operator]

Gary Russell commented

Same problem applies to peek() in the MessageGroupQueue - fetches the whole group and discard all except the first.

[spring-operator]

Artem Bilan commented

Hello
Let me offer my thoughts on the matter.
I don't agree about using JdbcPagingItemReader. It is used for scenarios where we need several rows at a time and when they can be not on first page. in the 'pollMessageFromGroup()' we need only one first row and now it's OK, because it makes return Message after first call rs.next(). However here we need check result of call removeMessageFromGroup(groupId, message);: we should return message only when it is realy removed from store in the current transaction. And if we add this check we'll solve the issue about competing consumers in the distributed environment like cluster. Look on it as 'JDBC global lock'. I'm going to link an issue about global lock, where is my comment about commit, which show prototype of that solution.

Nevertheless there is some improvement which we can take from:
org.springframework.jdbc.core.JdbcTemplate#applyStatementSettings. So, it will be enough to setup fetchSize & maxRows on that instance in the JdbcMessageStore or do it in the specific Statement for 'pollMessageFromGroup()'.

I agree about MessageGroupQueue#peek(), so there we need separate method in the MessageGroupStore similar to 'pollMessageFromGroup()', but without message removing.

Also here I have a question: what is the reason to have such method specification:
MessageGroup removeMessageFromGroup(Object groupId, Message<?> messageToRemove)?
Why it is necessary to fetch whole MessageGroup after each message's removal?
I think there are many scenarios when we get MessageGroup and iterate over its message and call removeMessageFromGroup()...
I propose just returning only message from it and as result of call this.removeMessage(messageToRemove.getHeaders().getId());

[spring-operator]

Artem Bilan commented

Maybe this issue is related to #5123