Open ChunkMessageChannelItemWriter for extension

[spring-projects-issues]

Wim Veldhuis opened BATCH-2651 and commented

The current implementation of ChunkMessageChannelItemWriter.waitForResults will not read more than maxWaitTimeouts replies from the reply queue, even when there are more due to a larger throttle-limit.

This causes the job to get status failed, and the remaining replies are left ontouched on the reply queue. The next job will then read these replies, and complain about being them for the wrong job, forcing that job to fail as well.

---

Affects: 4.0.0.M4

Reference URL: https://stackoverflow.com/questions/47227650/remote-batch-job-does-not-read-all-responses-in-afterstep-method

[spring-projects-issues]

Wim Veldhuis commented

PR in #554

[spring-projects-issues]

Michael Minella commented

The contract for this is pretty clear. Why is this a bug?

[spring-projects-issues]

Wim Veldhuis commented

The contract for this, in my opinion, is to read all outstanding replies but not to wait for longer than maxReadTimeouts doing so.

The implementation however will not read more than maxReadTimeouts replies and not wait for maximal maxReadTimeouts timeouts.
So in the case where the throttlelimit is larger than the maxReadTimeouts (which is the case in our context), the method will NEVER read all the replies and the job will always FAIL.

The implementation should not count succesful reads as timeouts as it currently does.

My implementation takes the contract to be that maxReadTimeouts successive timeouts are required before the method returns false.

[spring-projects-issues]

Michael Minella commented

Wim Veldhuis, A typical timeout is just that. A timeout. It doesn't matter what else is going on, if you say "timeout after X", that means execution is complete after X. This isn't an inactivity timeout (which is what you are proposing to convert it to). That would cause issues where processing windows are in play (since you wouldn't be able to guarantee that a job would finish in a given window).

[spring-projects-issues]

Wim Veldhuis commented

So the contract means to implement a guarantuee the job will either complete with in maxReadTimeouts*receiveTimeout OR be successfully completed ?

Suppose a case where maxReadTimeouts = 40 with throttle-limit=100, receiveTimeout 5000 and there are 80 outstanding replies when we hit the waitForResults.
Also assuming all replies are generated within 5 seconds and processing a reply takes 50ms.

This should result in a succesful job because retrieving and processing the 80 outstanding replies would take less than 6 seconds. Well within the contract of the maximal time I would want to wait. However, the job will fail after around 2,5 seconds because 40 replies are handled.

Also because the method is private, it cannot be overruled.

In my opinion something is wrong here. My solution might be incorrect, but the current implemenation also fails as it will never let a job succeed where the outstanding amount of replies is larger than the maxReadtimeout size.

[fmbenhassine]

I agree with Michael. Resetting the timeout each time can lead to a batch window being exceeded, which is not and should not be the case in the first place. The timeout should really be related to a time window, and not the number of attempts.

Also because the method is private, it cannot be overruled.

The class will be opened for extension to be able to override the default behaviour if needed.