wang-jiahua commented on PR #10659: URL: https://github.com/apache/rocketmq/pull/10659#issuecomment-5200044338
> **[P1] Preserve the suspend flag in the backoff record** @fuyou001 Good catch. Fixed in `771afdcd4` exactly as suggested: the backoff record now uses the nine-argument constructor and carries `record.isSuspend()` through, so the client intent expressed by `changeInvisibilityDuration(..., suspend=true)` survives a transient revive failure. To be precise about the origin: the eight-argument constructor hard-codes `suspend` to `false` when it delegates, and the batch backoff path has always used it, so losing the flag predates this PR. What this PR changed is the exposure: converting synchronous throws and exceptionally completed futures into `false` routes two additional failure classes into that same backoff path, which is why it is worth fixing here. The regression test `reviveBackoffRecordShouldPreserveSuspendFlag` covers both failure paths you asked for: one record fails synchronously and one asynchronously, both with `suspend=true`. It then reloads the persisted retry records from the store and asserts that `suspend` is still true and `attemptTimes` is incremented exactly once. Since `writeRecords(failureList)` runs before `deleteRecords(consumerRecords)`, the originals are only removed after the retry records are persisted. `reviveRetry` keeping `reconsumeTimes` unchanged for suspended records is already covered by the existing suspend tests. The assertion fails before this commit, with the logs showing `suspend=true` going in and `suspend=false` coming out. The same eight-argument constructor issue exists in the cache callback proposed for #10667, and that work preserves the flag as well. `PopConsumerServiceTest` is at 21/21 with zero Checkstyle violations on the current develop baseline. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
