Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #566: NotSerializableException: SessionRegistry$PublishedMessage #568

Closed

Conversation

hylkevds
Copy link
Collaborator

@hylkevds hylkevds commented Feb 1, 2021

Fixes #566

Instances of implementations of the interface
SessionRegistry$EnqueuedMessage are put in the queue, which can be
backed by the H2 store. But since this class is not serializable, H2
can not store it. This commit makes the interface serializable, and
deals with the non-serializable ByteBuf that the PublishedMessage
subclass has as a field.

…ishedMessage

Instances of implementations of the interface
SessionRegistry$EnqueuedMessage are put in the queue, which can be
backed by the H2 store. But since this class is not serializable, H2
can not store it. This commit makes the interface serializable, and
deals with the non-serializable ByteBuf that the PublishedMessage
subclass has as a field.
@andsel
Copy link
Collaborator

andsel commented Mar 27, 2021

@hylkevds thanks to point out this problem. I'm thinking if Java serializaion could be avoided, maybe encoding the overall object as a byte array of (topic bytes, q0s byte, message bytes), or maybe a use Protobuf to serialize it; what do you think?

@hylkevds
Copy link
Collaborator Author

Long term that's probably a better solution. Is the Serializable a result of the H2 store, or is possible to specify on the H2 store how the serialisation has to happen?

@andsel
Copy link
Collaborator

andsel commented Mar 28, 2021

I think that to use custom deserializer we should:

@hylkevds
Copy link
Collaborator Author

Yes, that should work. Probably also more efficient.
Using Protobuf isn't even really needed, since the read and write operations of DataType already pass ByteBuffers. The topic and message we need to store are already Bytes, and the qOs can also simply be stored as a byte. We can just write 4 (8?) length bytes for the topic, followed by the String bytes, 8 length bytes for the payload, followed by the payload and finally the qOs byte. That's essentially what I also did for the java serialization implementation.
That avoids pulling in more dependencies too.

@andsel
Copy link
Collaborator

andsel commented Mar 30, 2021

I agree with your idea, I'll try to put down a draft for this; I would move the fixed length field (QoS) in front, so in case of debug the byte stream is more straight forward.

@andsel andsel added this to the 0.14.1 milestone Apr 3, 2021
@andsel andsel closed this in #592 May 2, 2021
@andsel andsel added the v0.15.0 label May 8, 2021
@hylkevds hylkevds deleted the fix_566_nse_PublishedMessage branch February 2, 2022 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0.14: NotSerializableException: io.moquette.broker.SessionRegistry$PublishedMessage
2 participants