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

Recommendation for CompatibleFieldSerializer #684

Open
maxisvest opened this issue Aug 1, 2019 · 3 comments
Open

Recommendation for CompatibleFieldSerializer #684

maxisvest opened this issue Aug 1, 2019 · 3 comments
Labels

Comments

@maxisvest
Copy link

maxisvest commented Aug 1, 2019

I read the example in #643, and I see that in @NathanSweet last comment says when decode SomeClass#anotherClass but AnotherClass's structure is missing, it will cause decode failure. Because CompatibleFieldSerializer serialize is great depandency in order. When reading, the AnotherClass class doesn't exist, Kryo will skip AnotherClass 's chunked input buffer.
See CompatibleFieldSerializer#read's readUnknownTagData part down below.

Registration registration;
try {
	registration = kryo.readClass(fieldInput);
} catch (KryoException ex) {
	if (!chunked)
		throw new KryoException("Unable to read unknown data (unknown type). (" + getType().getName() + ")", ex);
	if (DEBUG) debug("kryo", "Unable to read unknown data (unknown type).", ex);
		inputChunked.nextChunk();
		continue;
}

But the information of SomeClass#value is still int that buffer, so if we continue read the remaing buffer that we can cache the SomeClass#value in SomeClass#anotherClass's chunked input.
So I simply do that:

Registration registration;
try {
	registration = kryo.readClass(fieldInput);
} catch (KryoException ex) {
	if (!chunked)
		throw new KryoException("Unable to read unknown data (unknown type). (" + getType().getName() + ")", ex);
	if (DEBUG) debug("kryo", "Unable to read unknown data (unknown type).", ex);
		//inputChunked.nextChunk();
		//continue;
               registration = kryo.getRegistration(Object.class);
}

If Kryo can't find the Class so we give it a default Class that has no field in it, then Kryo successfully read the remaing byte in buffer and cache the SomeClass#value in SomeClass#anotherClass's chunked input.

This change is usefull in my case, but I don't know it will have impact for other case? Or you consider add this change (maybe with configurable) in futrue release?

Looking forward to your reply.

@NathanSweet
Copy link
Member

Sorry for the very late response!

You lost me at:

But the information of SomeClass#value is still int that buffer,

Each field is written chunked, so when we can't read a field and inputChunked.nextChunk();, we only skip the bytes for that field.

Maybe you can explain more how you are using your proposed change?

@maxisvest
Copy link
Author

@NathanSweet

is still int that buffer

is a wrong spelling,I mean

But the information of SomeClass#value is still in that buffer

which means in serializingSomeClass#anotherClass chunk is contains SomeClass#value Class infomation and cache this information as a index to serialize SomeClass#value, but in deserializng, inputChunked.nextChunk() will skip this thing, so when deserialize to SomeClass#value, kryo find that the index it can not mapping or mismatch the true Class infomation.

This change is try to resolve this problem, force kryo read the next chunk is safely, if kryo.getRegistration(Object.class) throw an execption, that's mean this chunk is truly broken and should be skip, if not, we may cache some Class information which possibly used by the next few chunk.

This change is already in our project and works fine, so you can consider merge this PR #686

@NathanSweet
Copy link
Member

Sorry it's been so long.

While I don't have time right now to run the code in the PR, it doesn't appear valid to use the Object registration when kryo.readClass fails. Why would the serializer for Object be able to deserialize the bytes for some other class?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants