Hi tison, Yes, it should be as simple as you pointed out. I have open pr-2104[1] and pr-2105[2] for this.
[1]: https://github.com/apache/zookeeper/pull/2104 [2]: https://github.com/apache/zookeeper/pull/2105 Best, Kezhu Wang On Fri, Nov 10, 2023 at 8:27 PM Zili Chen <ti...@apache.org> wrote: > > Hi Kezhu, > > Thanks for bringing up this topic! > > The main issue seems to be maintenance considerations instead of user-facing > concerns. > > UPDATE - > > I found that we can simply edit `boolean validpacket = > Request.isValid(si.type);` to return false in OpCode.check. Then it's a good > trade-off to apply your proposal. +1. > > ORIGIN - > > If so, I wonder how we can distinguish individual requests from those from > TXN? > > Also, IIRC client API doesn't support sending individual requests with > OpCode.check at all. Perhaps we can make it an assertion so that nothing > should be changed. > > Best, > tison. > > On 2023/10/17 03:57:26 Kezhu Wang wrote: > > Hi devs, > > > > It is a long time issue that OpCode.check can cause client > > disconnection and server unusable or even database corruption. It has > > been reported twice in ZOOKEEPER-2623[1] and ZOOKEEPER-4680[2]. Both > > are reported by third-party client authors. > > > > I opened pr-1988[3](merged now) to make OpCode.check a pure read > > operation and return nothing to match OpResult.CheckResult in > > MultiResponse. But I am rethinking this when investigating > > ZOOKEEPER-4750[4] and wonder whether UNIMPLEMENTED is more appropriate > > for reasons: > > 1. OpCode.check was designed to function inside OpCode.multi but not > > individually. > > 2. We have OpCode.exists, it covers the OpCode.check behavior > > pr-1988[3] introduced. > > 3. It could confuse us when inspecting logs and metrics if we record > > operation statistics somewhere. This probably introduces a long term > > maintenance burden. > > > > So I am here to seek a consensus on this. I prefer to UNIMPLEMENTED > > even pr-1988[3] has been merged. > > > > @anmolnar might want to backport this to 3.8 and 3.9 series. I am > > supportive of backport. So I think we probably should align before > > backporting. > > > > Any thoughts ? > > > > [1]: https://issues.apache.org/jira/browse/ZOOKEEPER-2623 > > [2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4680 > > [3]: https://github.com/apache/zookeeper/pull/1988 > > [4]: https://issues.apache.org/jira/browse/ZOOKEEPER-4750 > >
