capistrant opened a new issue, #12999: URL: https://github.com/apache/druid/issues/12999
### Affected Version 0.22.2 ### Description ``` 2022-08-30T14:10:38,628 INFO [HttpServerInventoryView-6] org.apache.druid.server.coordination.ChangeRequestHttpSyncer - [https://XXXXXX.XXXXXX.com:8283/_1661264760506] requested resetCounter for reason [counter[Counter{counter=94953, hash=1661868638606}] >= last counter[Counter{counter=94953, hash=1661868638606}]]. ``` You can see here that the client request for changes on this historical provides a Counter that matches the last counter on the server. This causes the client to send a fresh request to the server with a null counter in order to get the full set of segments. I am struggling to understand why we would want to reset a server that is up to date. It looks like [this](https://github.com/apache/druid/blob/0.22.2/server/src/main/java/org/apache/druid/server/coordination/ChangeRequestHistory.java#L146) is what is causing the Counter to be reset. It seems that the above code path is run either when the Counter **does not match** when the client request is made OR when the waiting futures (requests waiting for a new change) are resolved [here](https://github.com/apache/druid/blob/0.22.2/server/src/main/java/org/apache/druid/server/coordination/ChangeRequestHistory.java#L197). It is this latter case that I don't understand the reset Counter return to client? Why not send some empty response and the client comes back with the same counter looking for a new change? Resetting it feels like overkill and added work for no gain. Perhaps I'm missing something here, but as of now it feels like incorrect, albeit not destructive, behavior. spit balling on change to [the code that controls resetting the Counter](https://github.com/apache/druid/blob/0.22.2/server/src/main/java/org/apache/druid/server/coordination/ChangeRequestHistory.java#L146) ```JAVA if (counter.counter == lastCounter.counter) { if (counter.matches(lastCounter)) { // return some empty response that does not cause client to reset counter } else { // this is bad, fail with a reset due to an incorrect hash - unlikely this would happen here, but it adds safety } } else if (counter.counter > lastCounter.counter) { // This is modification of original conditional to just GT return ChangeRequestsSnapshot.fail( StringUtils.format( "counter[%s] > last counter[%s]", counter, lastCounter ) ); } else if ... ``` -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
