You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by Lari Hotari <lh...@apache.org> on 2023/01/09 11:05:35 UTC

[DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Hi all,

Jackson has a separate Java 8 support modules for adding support for proper serialization and deserialization of new classes that were added in Java 8 (Java 8 was released in 2014). 

These Jackson Java 8 support modules haven't been used in the Pulsar code base. This is a pity. This causes a lot of pain when using Java Time classes in Pulsar applications or Pulsar Functions. There are ways to get the classes working for applications, but the documentation is missing. It would make things easier if the Java 8 support modules for Jackson would be included and registered by default.

I have created a PR to register Jackson Java 8 support modules by default for all Pulsar components. The PR is https://github.com/apache/pulsar/pull/19161 .

Please review and provide feedback. Do we need a PIP for this change?

-Lari

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Asaf Mesika <as...@gmail.com>.
This was a clear explanation. Thanks!

On Mon, Jan 23, 2023 at 10:40 AM Lari Hotari <lh...@apache.org> wrote:

> A potential breaking change would be the case when Java 8 support module
> hasn't been registered and Jackson seems to serialize and de-serialize the
> objects successfully, but with a obscure JSON structure.
>
> Jackson 2.12 introduced a solution to prevent this from happening for Java
> 8 date/time types. (Jackson change:
> https://github.com/FasterXML/jackson-databind/issues/2683)
> Jackson 2.12 was first used in Pulsar 2.8.0 (Jackson upgrade 2.11 -> 2.12
> in https://github.com/apache/pulsar/pull/10782).
>
> If you try to use Java 8 date/time types with Jackson, you will get this
> exception unless the Java 8 support modules are registered:
> com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Java 8
> date/time type `java.time.Instant` not supported by default: add Module
> "com.fasterxml.jackson.datatype:jackson-datatype-jsr310" to enable handling
>
> An example where Jackson doesn't fail at serialization time, is the usage
> of java.util.Optional . However, the value doesn't get serialized at all
> and deserialization fails.
>
> The risk of this being a breaking change is low since there doesn't seem
> to be cases where serialization and de-serialization would work without the
> Java 8 support modules using a bad JSON structure.
>
>
> > Context question since I'm not familiar with functions: How does a
> function
> > author use Jackson provided by Pulsar? Why does pulsar provide Jackson to
> > function?
>
> The Pulsar Functions will use JSON serialization/deserialization by
> default. (documented in
> https://pulsar.apache.org/docs/2.10.x/functions-develop/#serde ). The
> runtime uses Jackson for this purpose.
>
> -Lari
>
> On 2023/01/17 07:01:13 Asaf Mesika wrote:
> > Can the scenarios of breaking changes for users be described in the PIP
> if
> > they exist? If this is a breaking change, it might be a good thing to
> point
> > it out in the PIP and in the release notes ("If you did this, then
> perhaps
> > you want to do that").
> >
> > Context question since I'm not familiar with functions: How does a
> function
> > author use Jackson provided by Pulsar? Why does pulsar provide Jackson to
> > function?
> >
> >
> > On Mon, Jan 16, 2023 at 12:12 PM Lari Hotari <lh...@apache.org> wrote:
> >
> > > I created PIP-243, https://github.com/apache/pulsar/issues/19243 .
> Since
> > > this is already discussed in this thread, I'll open it directly for
> voting.
> > >
> > > -Lari
> > >
> > > On 2023/01/09 11:05:35 Lari Hotari wrote:
> > > > Hi all,
> > > >
> > > > Jackson has a separate Java 8 support modules for adding support for
> > > proper serialization and deserialization of new classes that were
> added in
> > > Java 8 (Java 8 was released in 2014).
> > > >
> > > > These Jackson Java 8 support modules haven't been used in the Pulsar
> > > code base. This is a pity. This causes a lot of pain when using Java
> Time
> > > classes in Pulsar applications or Pulsar Functions. There are ways to
> get
> > > the classes working for applications, but the documentation is
> missing. It
> > > would make things easier if the Java 8 support modules for Jackson
> would be
> > > included and registered by default.
> > > >
> > > > I have created a PR to register Jackson Java 8 support modules by
> > > default for all Pulsar components. The PR is
> > > https://github.com/apache/pulsar/pull/19161 .
> > > >
> > > > Please review and provide feedback. Do we need a PIP for this change?
> > > >
> > > > -Lari
> > > >
> > >
> >
>

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Lari Hotari <lh...@apache.org>.
A potential breaking change would be the case when Java 8 support module hasn't been registered and Jackson seems to serialize and de-serialize the objects successfully, but with a obscure JSON structure.

Jackson 2.12 introduced a solution to prevent this from happening for Java 8 date/time types. (Jackson change: https://github.com/FasterXML/jackson-databind/issues/2683)
Jackson 2.12 was first used in Pulsar 2.8.0 (Jackson upgrade 2.11 -> 2.12 in https://github.com/apache/pulsar/pull/10782).

If you try to use Java 8 date/time types with Jackson, you will get this exception unless the Java 8 support modules are registered:
com.fasterxml.jackson.databind.exc.InvalidDefinitionException: Java 8 date/time type `java.time.Instant` not supported by default: add Module "com.fasterxml.jackson.datatype:jackson-datatype-jsr310" to enable handling

An example where Jackson doesn't fail at serialization time, is the usage of java.util.Optional . However, the value doesn't get serialized at all and deserialization fails. 

The risk of this being a breaking change is low since there doesn't seem to be cases where serialization and de-serialization would work without the Java 8 support modules using a bad JSON structure.


> Context question since I'm not familiar with functions: How does a function
> author use Jackson provided by Pulsar? Why does pulsar provide Jackson to
> function?

The Pulsar Functions will use JSON serialization/deserialization by default. (documented in https://pulsar.apache.org/docs/2.10.x/functions-develop/#serde ). The runtime uses Jackson for this purpose. 

-Lari

On 2023/01/17 07:01:13 Asaf Mesika wrote:
> Can the scenarios of breaking changes for users be described in the PIP if
> they exist? If this is a breaking change, it might be a good thing to point
> it out in the PIP and in the release notes ("If you did this, then perhaps
> you want to do that").
> 
> Context question since I'm not familiar with functions: How does a function
> author use Jackson provided by Pulsar? Why does pulsar provide Jackson to
> function?
> 
> 
> On Mon, Jan 16, 2023 at 12:12 PM Lari Hotari <lh...@apache.org> wrote:
> 
> > I created PIP-243, https://github.com/apache/pulsar/issues/19243 . Since
> > this is already discussed in this thread, I'll open it directly for voting.
> >
> > -Lari
> >
> > On 2023/01/09 11:05:35 Lari Hotari wrote:
> > > Hi all,
> > >
> > > Jackson has a separate Java 8 support modules for adding support for
> > proper serialization and deserialization of new classes that were added in
> > Java 8 (Java 8 was released in 2014).
> > >
> > > These Jackson Java 8 support modules haven't been used in the Pulsar
> > code base. This is a pity. This causes a lot of pain when using Java Time
> > classes in Pulsar applications or Pulsar Functions. There are ways to get
> > the classes working for applications, but the documentation is missing. It
> > would make things easier if the Java 8 support modules for Jackson would be
> > included and registered by default.
> > >
> > > I have created a PR to register Jackson Java 8 support modules by
> > default for all Pulsar components. The PR is
> > https://github.com/apache/pulsar/pull/19161 .
> > >
> > > Please review and provide feedback. Do we need a PIP for this change?
> > >
> > > -Lari
> > >
> >
> 

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Asaf Mesika <as...@gmail.com>.
Can the scenarios of breaking changes for users be described in the PIP if
they exist? If this is a breaking change, it might be a good thing to point
it out in the PIP and in the release notes ("If you did this, then perhaps
you want to do that").

Context question since I'm not familiar with functions: How does a function
author use Jackson provided by Pulsar? Why does pulsar provide Jackson to
function?


On Mon, Jan 16, 2023 at 12:12 PM Lari Hotari <lh...@apache.org> wrote:

> I created PIP-243, https://github.com/apache/pulsar/issues/19243 . Since
> this is already discussed in this thread, I'll open it directly for voting.
>
> -Lari
>
> On 2023/01/09 11:05:35 Lari Hotari wrote:
> > Hi all,
> >
> > Jackson has a separate Java 8 support modules for adding support for
> proper serialization and deserialization of new classes that were added in
> Java 8 (Java 8 was released in 2014).
> >
> > These Jackson Java 8 support modules haven't been used in the Pulsar
> code base. This is a pity. This causes a lot of pain when using Java Time
> classes in Pulsar applications or Pulsar Functions. There are ways to get
> the classes working for applications, but the documentation is missing. It
> would make things easier if the Java 8 support modules for Jackson would be
> included and registered by default.
> >
> > I have created a PR to register Jackson Java 8 support modules by
> default for all Pulsar components. The PR is
> https://github.com/apache/pulsar/pull/19161 .
> >
> > Please review and provide feedback. Do we need a PIP for this change?
> >
> > -Lari
> >
>

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Lari Hotari <lh...@apache.org>.
I created PIP-243, https://github.com/apache/pulsar/issues/19243 . Since this is already discussed in this thread, I'll open it directly for voting.

-Lari

On 2023/01/09 11:05:35 Lari Hotari wrote:
> Hi all,
> 
> Jackson has a separate Java 8 support modules for adding support for proper serialization and deserialization of new classes that were added in Java 8 (Java 8 was released in 2014). 
> 
> These Jackson Java 8 support modules haven't been used in the Pulsar code base. This is a pity. This causes a lot of pain when using Java Time classes in Pulsar applications or Pulsar Functions. There are ways to get the classes working for applications, but the documentation is missing. It would make things easier if the Java 8 support modules for Jackson would be included and registered by default.
> 
> I have created a PR to register Jackson Java 8 support modules by default for all Pulsar components. The PR is https://github.com/apache/pulsar/pull/19161 .
> 
> Please review and provide feedback. Do we need a PIP for this change?
> 
> -Lari
> 

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Lari Hotari <lh...@apache.org>.
The task to optimize the code that calls Jackson is handled by PR https://github.com/apache/pulsar/pull/19160 as a preparation. Please review since I will merge that and consider lazy consensus unless changes are requested.

In the Pulsar master code base, we are already at Java 17. It's time to add support to Jackson support classes that were added in Java 8. The draft PR is https://github.com/apache/pulsar/pull/19161 (it will be rebased after PR 19160 changes are in). I can create a PIP to formalize and document the decision before pushing for a merge for that PR.

btw. The pulsar code base also includes GSON for some JSON parsing. It would be good to replace its usage with Jackson. I wonder if there's a special reason why GSON is used besides Jackson.

-Lari

On 2023/01/10 13:13:50 "rxl@apache.org" wrote:
> Hello Lari:
> 
> Here I actually want to discuss whether it is necessary for us to introduce
> Jackson modules, and what is the scope boundary we use when introducing
> Jackson modules? Here are some questions about this:
> 
> 
> 1. After we introduce Jackson modules, do we need to optimize the code that
> originally called Jackson?
> 2. After the introduction, we keep the original code using Jackson
> unchanged, and only use the logic of Jackson modules in the newly added code
> 
> If we take 1, it seems that the changes here will be relatively large. Many
> Pojo objects will involve the serialization and deserialization of Json
> objects, and it feels that the entire boundary is not well controlled.
> 
> +1 for me if we're taking 2.
> 
> --
> Thanks
> Xiaolong Ran
> 
> Lari Hotari <lh...@apache.org> 于2023年1月9日周一 21:25写道:
> 
> > Any change to Jackson configuration is a potential breaking change. Yes,
> > it will need a PIP.
> > I guess we can continue to discuss the change in this thread before I
> > create an actual PIP which can be voted on.
> >
> > -Lari
> >
> > On 2023/01/09 11:53:02 丛搏 wrote:
> > > Hi, Lari:
> > >
> > > Will it affect compatibility? If it is just an improved function, I
> > > think it can also be added to the pulsar-common module. it adds the
> > > dependency, so it needs PIP to discuss.
> > >
> > > Thanks,
> > > Bo
> > >
> > > Lari Hotari <lh...@apache.org> 于2023年1月9日周一 19:06写道:
> > > >
> > > > Hi all,
> > > >
> > > > Jackson has a separate Java 8 support modules for adding support for
> > proper serialization and deserialization of new classes that were added in
> > Java 8 (Java 8 was released in 2014).
> > > >
> > > > These Jackson Java 8 support modules haven't been used in the Pulsar
> > code base. This is a pity. This causes a lot of pain when using Java Time
> > classes in Pulsar applications or Pulsar Functions. There are ways to get
> > the classes working for applications, but the documentation is missing. It
> > would make things easier if the Java 8 support modules for Jackson would be
> > included and registered by default.
> > > >
> > > > I have created a PR to register Jackson Java 8 support modules by
> > default for all Pulsar components. The PR is
> > https://github.com/apache/pulsar/pull/19161 .
> > > >
> > > > Please review and provide feedback. Do we need a PIP for this change?
> > > >
> > > > -Lari
> > >
> >
> 

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by "rxl@apache.org" <ra...@gmail.com>.
Hello Lari:

Here I actually want to discuss whether it is necessary for us to introduce
Jackson modules, and what is the scope boundary we use when introducing
Jackson modules? Here are some questions about this:


1. After we introduce Jackson modules, do we need to optimize the code that
originally called Jackson?
2. After the introduction, we keep the original code using Jackson
unchanged, and only use the logic of Jackson modules in the newly added code

If we take 1, it seems that the changes here will be relatively large. Many
Pojo objects will involve the serialization and deserialization of Json
objects, and it feels that the entire boundary is not well controlled.

+1 for me if we're taking 2.

--
Thanks
Xiaolong Ran

Lari Hotari <lh...@apache.org> 于2023年1月9日周一 21:25写道:

> Any change to Jackson configuration is a potential breaking change. Yes,
> it will need a PIP.
> I guess we can continue to discuss the change in this thread before I
> create an actual PIP which can be voted on.
>
> -Lari
>
> On 2023/01/09 11:53:02 丛搏 wrote:
> > Hi, Lari:
> >
> > Will it affect compatibility? If it is just an improved function, I
> > think it can also be added to the pulsar-common module. it adds the
> > dependency, so it needs PIP to discuss.
> >
> > Thanks,
> > Bo
> >
> > Lari Hotari <lh...@apache.org> 于2023年1月9日周一 19:06写道:
> > >
> > > Hi all,
> > >
> > > Jackson has a separate Java 8 support modules for adding support for
> proper serialization and deserialization of new classes that were added in
> Java 8 (Java 8 was released in 2014).
> > >
> > > These Jackson Java 8 support modules haven't been used in the Pulsar
> code base. This is a pity. This causes a lot of pain when using Java Time
> classes in Pulsar applications or Pulsar Functions. There are ways to get
> the classes working for applications, but the documentation is missing. It
> would make things easier if the Java 8 support modules for Jackson would be
> included and registered by default.
> > >
> > > I have created a PR to register Jackson Java 8 support modules by
> default for all Pulsar components. The PR is
> https://github.com/apache/pulsar/pull/19161 .
> > >
> > > Please review and provide feedback. Do we need a PIP for this change?
> > >
> > > -Lari
> >
>

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by Lari Hotari <lh...@apache.org>.
Any change to Jackson configuration is a potential breaking change. Yes, it will need a PIP.
I guess we can continue to discuss the change in this thread before I create an actual PIP which can be voted on.

-Lari

On 2023/01/09 11:53:02 丛搏 wrote:
> Hi, Lari:
> 
> Will it affect compatibility? If it is just an improved function, I
> think it can also be added to the pulsar-common module. it adds the
> dependency, so it needs PIP to discuss.
> 
> Thanks,
> Bo
> 
> Lari Hotari <lh...@apache.org> 于2023年1月9日周一 19:06写道:
> >
> > Hi all,
> >
> > Jackson has a separate Java 8 support modules for adding support for proper serialization and deserialization of new classes that were added in Java 8 (Java 8 was released in 2014).
> >
> > These Jackson Java 8 support modules haven't been used in the Pulsar code base. This is a pity. This causes a lot of pain when using Java Time classes in Pulsar applications or Pulsar Functions. There are ways to get the classes working for applications, but the documentation is missing. It would make things easier if the Java 8 support modules for Jackson would be included and registered by default.
> >
> > I have created a PR to register Jackson Java 8 support modules by default for all Pulsar components. The PR is https://github.com/apache/pulsar/pull/19161 .
> >
> > Please review and provide feedback. Do we need a PIP for this change?
> >
> > -Lari
> 

Re: [DISCUSS] Registering Jackson Java 8 support modules by default for all Pulsar components, including client

Posted by 丛搏 <bo...@apache.org>.
Hi, Lari:

Will it affect compatibility? If it is just an improved function, I
think it can also be added to the pulsar-common module. it adds the
dependency, so it needs PIP to discuss.

Thanks,
Bo

Lari Hotari <lh...@apache.org> 于2023年1月9日周一 19:06写道:
>
> Hi all,
>
> Jackson has a separate Java 8 support modules for adding support for proper serialization and deserialization of new classes that were added in Java 8 (Java 8 was released in 2014).
>
> These Jackson Java 8 support modules haven't been used in the Pulsar code base. This is a pity. This causes a lot of pain when using Java Time classes in Pulsar applications or Pulsar Functions. There are ways to get the classes working for applications, but the documentation is missing. It would make things easier if the Java 8 support modules for Jackson would be included and registered by default.
>
> I have created a PR to register Jackson Java 8 support modules by default for all Pulsar components. The PR is https://github.com/apache/pulsar/pull/19161 .
>
> Please review and provide feedback. Do we need a PIP for this change?
>
> -Lari