pkuwm commented on pull request #1072: URL: https://github.com/apache/helix/pull/1072#issuecomment-641628818
> I'm not convinced that this is a good idea. > > For the non-Helix user, ZNRecord is not meaningful. How do you plan to document this config and let them configure ZK_SERIALIZER_**ZNRECORD**_WRITE_SIZE_LIMIT_BYTES for the general ZK usage? > Secondly, we shall not block the write in the ZkClient if the write size is smaller than the jute.maxbuffer configuration. That is not very meaningful. On the other hand, if we have a large ZK_SERIALIZER_ZNRECORD_WRITE_SIZE_LIMIT_BYTES configured, but the jute.maxbuffer is small, what we shall do? The write will still fail. > > My suggestion, > > * The raw ZkClient shall check size based on jute.maxbuffer, and maybe we leave some buffer like the default limit is current doing. > * If jute.maxbuffer is not configured, then we apply the default value according to the ZK document. @jiajunwang Thanks for the comments. I have the same concern about using the same config at zkclient level. This is why I would like to have it discussed at #1071 and confirmed: > A good idea is to make both consistently same. But the system property zk.serializer.znrecord.write.size.limit.bytes is for a ZNRecord serializer, not sure if it is fine to put it at ZkClient level, as it will apply to all other serializers, as well. We may discuss this. I have one thought if we use `jute.maxbuff` value here. If `jute.maxbuff` is set, considering client and server's values are the same. zkclient might be able to write but fail to read, due to the extra fields in ZK server. This is why we want to leave some buffer. But it may be difficult to determine the buffer size. Say we leave 5 KB as buffer, the value would be maxbuffer - 5 KB. But the value of ZK_SERIALIZER_**ZNRECORD**_WRITE_SIZE_LIMIT_BYTES might fall in the range of [maxbuffer, maxbuffer - 5 KB]. Even serializer check passes but it could be blocked by this check maxbuffer - 5 KB. If we are using jute.maxbuffer for this check, we have to document to make it clear ZK_SERIALIZER_ZNRECORD_WRITE_SIZE_LIMIT_BYTES should be less or equal to maxbuffer - 5KB. If possible, I would prefer to just have one config `zkclient.znode.write.size.limit` at ZkClient level, instead of znrecord serializer. So it satisfies all cases. Let's discuss further. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
