OneSizeFitsQuorum commented on PR #926: URL: https://github.com/apache/ratis/pull/926#issuecomment-1732839592
I'm pondering why we should update `nextIndex` when we receive a successful response. At this point, the GrpcLogAppender thread's streaming send may have already sent it to a larger index. If we reduce `nextIndex` in response to a smaller `matchIndex + 1`, even we fix this exception in this pr, it would still lead to the retransmission of many logs that have already been sent. I checked the example in the [issue](https://issues.apache.org/jira/browse/RATIS-1883) that introduced this problem, and I feel that the 7th step in the middle might be a typo; it should probably indicate that it was log [5]. <img width="1382" alt="image" src="https://github.com/apache/ratis/assets/32640567/b1d9ccbb-6052-42e9-95aa-ca7ff0657dcc";> As far as I recall, gRPC's streaming interface can guarantee sequential semantics, so there shouldn't be a situation where responses sent later arrive earlier. Therefore, could it be that the example in that issue doesn't exist actually? If the above statement is correct, maybe we can still lower nextIndex as before, only when the snapshot is transferred or some inconsistency is found, and we can leave nextIndex unchanged when we receive a correct response. -- 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]
