[FLINK-10065] InstantiationUtil.deserializeObject(InputStream in, ClassLoader cl, boolean isFailureTolerant) will close the inputStream

[klion26]

What is the purpose of the change

fix auto close InputStram in InstantiationUtil.deserializeObject

Verifying this change

This change is already covered by existing tests.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): no
• The public API, i.e., is any changed class annotated with @Public(Evolving): no
• The serializers: no
• The runtime per-record code paths (performance sensitive): no
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: no
• The S3 file system connector: no

Documentation

• Does this pull request introduce a new feature? no

[klion26]

cc @tillrohrmann @fhueske @tzulitai

[StephanEwen]

Interesting. This looks correct. The comment in the code even says the stream should not be closed.

[StephanEwen]

Seems correct, especially because the comment above that part indicates that the stream should not be closed.

@tzulitai can you share why this was changes in 9659ffd ?

Are there other places that call this functions where we now need to close the stream explicitly?

[klion26]

@StephanEwen I tracked all the routes, except the following one place may be need close explicity,all the other cals are in try-with-resource or use finally to close the stream explicitly.

InternalTimersSnapshotReaderWriters.InternalTimersSnapshotReaderPreVersioned#restoreKeyAndNamespaceSerializers

Could you please help checking this, If this should close explicitly, I will add it.

[StefanRRichter]

@klion26 The mentioned method is a legacy code path that will be removed eventually. I would not worry too much about that case.

[klion26]

ping @StephanEwen @tzulitai @tillrohrmann

[klion26]

Hi, @StephanEwen @tzulitai @tillrohrmann Could you help to review this pr please?

I encountered this problem while refactor snapshot/restore in my owner branch, in the restore function, I opened the file and called InstantiationUtil#deserializeObject from the previous opened InputStream, then the InputStream will closed by InstantiationUtil#deserializeObject.

I think others may encounter this problem too, so please help to review this pr, thank you

[tillrohrmann]

@StefanRRichter it seems to me that this method is still being used by the current code.

Before continuing with this PR I'd like to hear @tzulitai whether this was changed deliberately or not.

[StefanRRichter]

@tillrohrmann that is possible, my comment was just about InternalTimersSnapshotReaderPreVersioned#restoreKeyAndNamespaceSerializers

[tzulitai]

@tillrohrmann I don't think this was done deliberately, and the changes here looks correct.

[klion26]

@tillrohrmann , @StefanRRichter , @tzulitai thank you all for replying, is there anything I need to do before continuing?