patsonluk commented on PR #1438: URL: https://github.com/apache/solr/pull/1438#issuecomment-1714685834
Closing this as the proposed change can introduce incorrect behavior. For example, if there's already pending changes in the `ZkStateWriter`, and a no op changes invocation to `enqueueUpdate` can invoke the callback while the actual pending changes are not flushed. The existing behavior (shortcut no-op/empty ops and skip callback) is not ideal but is technically correct, as the `ZkWriteCallback#onWrite` method has description of `Called by ZkStateWriter if state is flushed to ZK`, since the empty/no-op operation does not trigger any state flush, it should not invoke the callback neither. This callback might not work in certain edge case, for example if there are a mix of both PRS and non PRS enabled collection, a PRS related operation might invoke `ZkWriteCallback#onWrite` while the operations for non PRS collections are still pending. However, this is probably a rather unlikely case and is beyond the scope of this PR -- 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]
