[FLINK-31127] Add public API classes for FLIP-289

[lindong28]

What is the purpose of the change

Add public API classes for FLIP-289 with the exception specified below.

Brief change log

Added most public API classes specified in FLIP-289.

The following class and methods are not added:

• GraphModel#supportServable
• PipelineModel#supportServable
• PipelineModelServable

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): yes

Documentation

• Does this pull request introduce a new feature? yes
• If yes, how is the feature documented? JavaDocs

[lindong28]

@jiangxin369 Can you help review this PR?

[jiangxin369]

@lindong28 Thanks for the PR, left some comments.

[jiangxin369]

1. Since We need to support setModelData on GraphModel, shall we modify the parameter to InputStream... modelDatas to support reading multiple model data from different Tables?
2. Since we usually need to read and deserialize the model data, we'd better add throws IOException in the method signature.

[lindong28]

Good point. I agree we need to make setModelData() take multiple InputSreams. And we need to let these methods throw IOException.

I have updated the method to the this:
default T setModelData(InputStream... modelDataInputs) throws IOException

I will send an email update to the voting thread when this PR is mostly ready for merge.

[lindong28]

@jiangxin369 Thanks for the review. Can you take another look?

[jiangxin369]

@lindong28 Thanks for the updates. LGTM overall only left 2 minor comments.

[lindong28]

@zhipeng93 Can you help review this PR? If this PR looks good to you, I will need to send an email to the FLIP-289 voting thread to discuss the following API change before this PR is merged.

Update ModelServable#setModelData to use the signature default T setModelData(InputStream... modelDataInputs) throws IOException.

[zhipeng93]

Thanks for the PR. Left some comments below.

[lindong28]

@zhipeng93 Thanks for the review. Can you help take another look?

[zhipeng93]

Thanks for the update. LGTM.

[Fanoid]

Update ModelServable#setModelData to use the signature default T setModelData(InputStream... modelDataInputs) throws IOException.

Hi, I just found we have made this change. I have a question: how do we obtain the corresponding input stream for each GraphNode in the GraphModel? modelDataInputs are linearly organized, but GraphNode are in a form of DAG.

It seems additional parameters are required in the signature of setModelData to help map InputStreams to GraphNodes.

[jiangxin369]

It seems additional parameters are required in the signature of setModelData to help map InputStreams to GraphNodes.

IMO, indeed we need to map InputStreams to GraphNodes, but it is implementation, not required in API. The existing GraphModel class has shown a way to implement it.

[Fanoid]

It seems additional parameters are required in the signature of setModelData to help map InputStreams to GraphNodes.

IMO, indeed we need to map InputStreams to GraphNodes, but it is implementation, not required in API. The existing GraphModel class has shown a way to implement it.

If not specifying the mapping in this method, we may need another public API to obtain the orders of GraphNodes. Otherwise, end-users can't figure out how to put the model data of all GraphNodes into an array/list.

[lindong28]

Hi, I just found we have made this change. I have a question: how do we obtain the corresponding input stream for each GraphNode in the GraphModel? modelDataInputs are linearly organized, but GraphNode are in a form of DAG.

It seems additional parameters are required in the signature of setModelData to help map InputStreams to GraphNodes.

@Fanoid we will need to add extra APIs in GraphBuilder and extra class GraphModelServable in order to compose multiple Estimator/Transformer into one Servable. We can obtain the input stream for ModelServable#setModelData similar to how the existing GraphBuilder obtain input streams for Model#setModelData. The high level idea is to use placeholder to provide this information to GraphBuilder. Maybe checkout testGraphWithSetGetModelData() [1] for example.

[1] https://github.com/apache/flink-ml/blob/master/flink-ml-core/src/test/java/org/apache/flink/ml/api/GraphTest.java#L213

[Fanoid]

@lindong28 Thanks for your explanation. I missed the point that we already have the orders in the GraphBuilder#buildEstimator. Then, I'm OK with this change.

[lindong28]

Thanks @zhipeng93 @Fanoid @jiangxin369 for the review.