kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anastasia Vela <av...@confluent.io>
Subject Re: [DISCUSS] KIP-673: Emit JSONs with new auto-generated schema
Date Tue, 29 Sep 2020 18:35:46 GMT
Hi Tom,

I'm glad it makes more sense. I modified the KIP just to make it a little
more clear as well.

Is the concern that deserialization would have no guarantee as to the order
it appears? Since ObjectNodes hold the tree structure in a LinkedHashMap,
the iterating order to deserialize would be defined in the order it was
inserted into the map.

I understand the performance concern. However, this would only be logged
when the trace level logging is turned on, so this logging isn't done
otherwise. Plus, the existing text-based logging is already very expensive,
so it might not be that significant of a change.

Thanks,
Anastasia

On Mon, Sep 28, 2020 at 3:06 AM Tom Bentley <tbentley@redhat.com> wrote:

> Hi Anastasia,
>
> Thanks for those changes. I eventually figured out that the
> XYZJsonDataConverter are the new classes from the factoring-out of the
> Jackson dependency (KAFKA-10384), after which the proposal made much more
> sense to me. Apologies for being slow on the uptake there.
>
> I can see now that property order in the JSON in the log would be
> determined by the declared field in the RPC JSON definition. That might not
> always be ideal since it can be hard to visually parse large unindented
> JSON, but maybe that could be dealt with once we knew there was a real
> problem.
>
> It's an implementation detail, but I wonder whether constructing a whole
> tree of JsonNodes might cause performance issues. It would be more work,
> but the XYZJsonDataConverter could be generated to have a method which took
> a JsonGenerator, thus avoiding the need to instantiate the nodes just for
> the purposes of logging.
>
> Kind regards,
>
> Tom
>
> On Fri, Sep 25, 2020 at 7:05 PM Anastasia Vela <avela@confluent.io> wrote:
>
> > Hi Tom,
> >
> > Thanks for your input!
> >
> > 1. I'll add more details for the RequestConvertToJson and
> > XYZJsonDataConverter classes. Hopefully it will be more clear, but just
> to
> > answer your question, RequestConvertToJson does not return a
> > XYZJsonDataConverter, but rather it returns a JsonNode which will be
> > serialized. The JsonDataConverter is the new auto-generated schema for
> each
> > request/response type that contains the method to return the JsonNode to
> be
> > serialized.
> >
> > 2. There is no defined order of the properties, rather it's in the order
> > that it is set in. So if you first set key B, then key A, the properties
> > would appear with key B first. JsonNodes when serialized does not sort
> the
> > keys.
> >
> > 3. Yes, serialization is done via Jackson databind.
> >
> > Thanks again,
> > Anastasia
> >
> > On Fri, Sep 25, 2020 at 1:15 AM Tom Bentley <tbentley@redhat.com> wrote:
> >
> > > Hi Anastasia,
> > >
> > > Thanks for the KIP, I can certainly see the benefit of this. I have a
> few
> > > questions:
> > >
> > > 1. I think it would be helpful to readers to explicitly illustrate the
> > > RequestConvertToJson and XYZJsonDataConverter classes (e.g. with method
> > > signatures for one or two methods), because currently it's not clear
> (to
> > me
> > > at least) exactly what's being proposed. Does the RequestConvertToJson
> > > return a XYZJsonDataConverter?
> > >
> > > 2. Does the serialization have a defined order of properties
> (alphabetic
> > > perhaps)? My concern here is that properties appearing in order
> according
> > > to how they are iterated in a hash map might harm human readability of
> > the
> > > logs.
> > >
> > > 3. Would the serialization be done via the Jackson databinding?
> > >
> > > Many thanks,
> > >
> > > Tom
> > >
> > > On Thu, Sep 24, 2020 at 11:49 PM Anastasia Vela <avela@confluent.io>
> > > wrote:
> > >
> > > > Hi all,
> > > >
> > > > I'd like to discuss KIP-673:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-673%3A+Emit+JSONs+with+new+auto-generated+schema
> > > >
> > > > This is a proposal to change the format of request and response
> traces
> > to
> > > > JSON, which would be easier to load and parse, because the current
> > format
> > > > is only JSON-like and not easily parsable.
> > > >
> > > > Let me know what you think,
> > > > Anastasia
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message