[
https://issues.apache.org/jira/browse/BOOKKEEPER-582?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14019110#comment-14019110
]
Sijie Guo commented on BOOKKEEPER-582:
--------------------------------------
Thanks [~ikelly] for the summary. [~fpj] I would reply based on this summary.
> Use of an enum for flags
the point that Ivan made is to use optional boolean. my point is that using
optional boolean, you need to do lots of negative cases checkings when you are
adding more booleans. I would prefer a more specific way to figure out what
flags are set, rather than doing lots of negative cases checking from implicit
optional booleans.
> Sending error messages twice
the error code. one is for packet while the other while is for individual
entries. the error code on individual entries are for cases that need to
distinguish the status of individual one. they are same from semantic, so there
is no reason to split to two set of error codes. as the splitting would
introduce complexity of maintaining the codes, and mapping them back to return
code in callbacks. I think it is similar as individual status codes in a
zookeeper transaction.
> Use of enum for errors
I would prefer using protobuf only for wire-level packet format backward
compatibility. but for application level semantic backward compatibility, I
would prefer a explicit way to produce responses back to old version clients
for backward compatibility. Although it might introduce codes for this, it is
clear and less risky. If we agree that servers should respond responses
according to the version of the protocol that old client speaks, then it
doesn't matter it is using enum or integer.
> Too much required.
I think the key point here is about the required field on ledgerId and entryId.
my point here is the whole bookie (either storage, protocol) is based on single
entry read/write.
- if you stick on current bookie storage design, the single entry read/write
are primitives of bookie protocol. so make entryId and ledgerId should be
required. all other protocol improvements (e.g. streaming reads, batch reads,
multiple writes) could be added based on these primitives.
- if you are going to redesign the whole bookie storage to change the
fundamental fact, it is better to a new request type to fit in your new bookie
storage, rather than patching the existing requests type to make it
complicated.
so the required fields are there based on whole bookkeeper design, they are not
blocking the future enhancements.
> Make bookie and client use protobuf for requests (non-wire part)
> ----------------------------------------------------------------
>
> Key: BOOKKEEPER-582
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-582
> Project: Bookkeeper
> Issue Type: Sub-task
> Components: bookkeeper-client, bookkeeper-server
> Reporter: Ivan Kelly
> Assignee: Aniruddha
> Fix For: 4.3.0
>
> Attachments:
> 0002-BOOKKEEPER-582-Make-bookie-and-client-use-protobuf-f.patch,
> 0002-BOOKKEEPER-582-Make-bookie-and-client-use-protobuf-f.patch,
> 0002-BOOKKEEPER-582-Make-bookie-and-client-use-protobuf-f.patch,
> 0003-BOOKKEEPER-582-Make-bookie-and-client-use-protobuf-f.patch,
> BOOKKEEPER-582.diff, BOOKKEEPER-582.v2.diff, BOOKKEEPER-582.v3.diff
>
>
> Make the client and the bookie use protobufs internally. This is the first
> step to using protobufs on the wire, but for the moment, BookieRequestHandler
> decodes the old wire protocol into the protobuf messages. Once this is in,
> enabling on the wire will be very simple, and the old manual serialization
> can be made "legacy" (still supported, but deprecated).
--
This message was sent by Atlassian JIRA
(v6.2#6252)
