[
https://issues.apache.org/jira/browse/BOOKKEEPER-582?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14042815#comment-14042815
]
Flavio Junqueira commented on BOOKKEEPER-582:
---------------------------------------------
I actually like the way the patch is done. I also prefer to use enums rather
than booleans. I agree though that it would be nice to send a bit vector of
flags rather than a single enum to avoid the combinatorial explosion. We should
check how other people have done with protobufs, I'm sure we are not the first
to encounter this problem. I did a bit of search, but couldn't find anything.
I'm also good with separating "packet-level errors" from "app-level errors".
I'm just not sure I agree with the codes in StatusCodes. For example, I don't
think "NOLEDGER" is a packet-level error. A packet-level error (or differently
stated, a server error as opposed to application error) is one that reflects a
problem that didn't allow the request to complete. No ledger means that the
request execution has completed, but it turns out that the ledger doesn't
exist, so report it. This is different from say having an IO exception that
prevents the processing of the request from completing. I think that if we
could separate those a bit better, then it would make more sense the separation.
About the use of enum for errors, I was checking the code and the valueOf
method it produces has a default value, which is null. I haven't actually
checked. If the default value propagates up, then we can spot that there is an
unknown value possibly because of a newer version. I haven't checked, though,
how it works on the decoder side.
The last point of "too much required" is a bit obscure to me, I'm not sure what
to do with it, so I'm ignoring.
If you just want a straight vote, then I'd stick to my previous +1, but my
preference is to work a bit on those error codes and perhaps investigate if
protobufs has a better way of dealing with enums and flags. It doesn't sound
hard to take an enumset and transform it into a bit vector, so maybe protobufs
should support it or provide similar functionality.
> 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)
