Hi Shengkai,
Thanks a lot for your comments! Please see my comments inline.
> 1. The FLIP does not specify the kind of SQL that will be submitted with
> the application mode. I believe only a portion of the SQL will be delegated
> to the SqlRunner.
You’re right. For SQL Gateway, only DMLs need to be delegated to the SQL
Driver. I would think about the details and update the FLIP. Do you have some
ideas already?
> 2. Will the SQL Client/Gateway perform any validation before submitting the
> SQL to the SqlRunner? If the user's SQL is invalid, it could take a long
> time to fetch error messages before execution.
Yes, I think so. We could introduce a SQL Gateway option like
`sql-gateway.application.validate-sql`, which determines if the Gateway
should do the SQL validation by compiling the plan.
> 3. ExecNodeGraph VS SQL File
ExecNodeGraph looks good to me. It’s not efficient to rebuild the
same TableEnvironment from scratch with SQL files at JobManager.
ExecNodeGraph avoids that.
Then I think SQL Driver should accept SQL files and ExecNodeGraph,
while the former is for public usage and the latter is for internal usage.
> 4. SqlRunner VS ApplicationRunner
ApplicationRunner looks good for SQL Driver at first glance, but
I’m afraid it has two limitations:
1) IIUC, ApplicationRunner should only be used in application mode, but
SQL Driver actually could be used in other deployment modes as well.
2) As FLIP-85 said, ApplicationRunner doesn’t work well with YARN.
What do you think?
> 5. K8S Application mode
WRT K8s application mode, I think it’s more of the users’ responsibility to
prepare the basic resources by customizing the image.
For non-SQL-gateway scenarios, the image should contain everything
needed for the SQLs.
For SQL-gateway scenarios, that’s actually a missing part that we need to
discuss. The gateway-generated resources (e.g. ExecNodeGraph) can’t be
accessed at JobManager, unless:
1) use web submission to deploy and run jars
2) mount a distributed storage to both SQL gateway and all nodes that
Flink runs on
I lean toward the web submission way but didn’t think through the details yet.
BTW, it seems that we don’t need `pipeline.jars` on K8s, we could put the
jars into the classpath directly.
> 6. Could you add more details about your modification in the gateway side,
> including error handling, execution workflow, and the impact on job
> statements?
Sure. I’ll let you know when I finish the details.
Paul Lam
> 2023年5月31日 10:56，Shengkai Fang <fskm...@gmail.com> 写道：
>
> Thanks for the proposal. The Application mode is very important to Flink
> SQL. But I have some questions about the FLIP:
>
> 1. The FLIP does not specify the kind of SQL that will be submitted with
> the application mode. I believe only a portion of the SQL will be delegated
> to the SqlRunner.
> 2. Will the SQL Client/Gateway perform any validation before submitting the
> SQL to the SqlRunner? If the user's SQL is invalid, it could take a long
> time to fetch error messages before execution.
> 3. ExecNodeGraph VS SQL File
>
> Initially, we planned to use ExecNodeGraph as the essential information to
> be submitted. By enabling 'table.plan.compile.catalog-objects' = 'all', the
> ExecNodeGraph provides necessary information about the session state.
> ExecNodeGraph contains the basic structure of tables and the class name of
> catalog functions used, which enables us to avoid serializing the catalog
> to the remote. Therefore, we prefer to use ExecGraph as the content
> submitted. Furthermore, our internal implementation can extend ExecGraph
> beyond state compatibility.
>
> 4. SqlRunner VS ApplicationRunner
>
> In the FLIP-85, it mentions to support Library mode. Compared to adding a
> new module, I think it's better to extend the origin design? WDYT?
>
> ApplicationRunner.run((StreamExecutionEnvironment env) -> {
>
> … // a consumer of the env
>
> })
>
> 5. K8S Application mode
>
> As far as I know, K8S doesn't support shipping multiple jars to the remote.
> It seems the current design also doesn't support K8S Application mode?
>
> https://github.com/apache/flink/blob/master/flink-kubernetes/src/main/java/org/apache/flink/kubernetes/KubernetesClusterDescriptor.java#L217
>
> 6. Could you add more details about your modification in the gateway side,
> including error handling, execution workflow, and the impact on job
> statements?
>
> Best,
> Shengkai
>
> Shammon FY <zjur...@gmail.com> 于2023年5月31日周三 10:40写道：
>
>> Thanks Paul for driving this proposal.
>>
>> I found the sql driver has no config related options. If I understand
>> correctly, the sql driver can be used to submit sql jobs in a 'job
>> submission service' such as sql-gateway. In general, in addition to the
>> default config for Flink cluster which includes k8s, ha and .etc, users may
>> also specify configurations for SQL jobs, including parallelism, number of
>> Task Managers, etc.
>>
>> For example, in sql-gateway users can `set` dynamic parameters before
>> submitting a sql statement, and for sql files users may put their
>> configurations in a `yaml` file. Should sql driver support dynamic
>> parameters and specify config file?
>>
>> Best,
>> Shammon FY
>>
>> On Tue, May 30, 2023 at 9:57 PM Weihua Hu <huweihua....@gmail.com> wrote:
>>
>>> Thanks Paul for the proposal.
>>>
>>> +1 for this. It is valuable in improving ease of use.
>>>
>>> I have a few questions.
>>> - Is SQLRunner the better name? We use this to run a SQL Job. (Not
>> strong,
>>> the SQLDriver is fine for me)
>>> - Could we run SQL jobs using SQL in strings? Otherwise, we need to
>> prepare
>>> a SQL file in an image for Kubernetes application mode, which may be a
>> bit
>>> cumbersome.
>>> - I noticed that we don't specify the SQLDriver jar in the
>>> "run-application"
>>> command. Does that mean we need to perform automatic detection in Flink?
>>>
>>>
>>> Best,
>>> Weihua
>>>
>>>
>>> On Mon, May 29, 2023 at 7:24 PM Paul Lam <paullin3...@gmail.com> wrote:
>>>
>>>> Hi team,
>>>>
>>>> I’d like to start a discussion about FLIP-316 [1], which introduces a
>> SQL
>>>> driver as the
>>>> default main class for Flink SQL jobs.
>>>>
>>>> Currently, Flink SQL could be executed out of the box either via SQL
>>>> Client/Gateway
>>>> or embedded in a Flink Java/Python program.
>>>>
>>>> However, each one has its drawback:
>>>>
>>>> - SQL Client/Gateway doesn’t support the application deployment mode
>> [2]
>>>> - Flink Java/Python program requires extra work to write a non-SQL
>>> program
>>>>
>>>> Therefore, I propose adding a SQL driver to act as the default main
>> class
>>>> for SQL jobs.
>>>> Please see the FLIP docs for details and feel free to comment. Thanks!
>>>>
>>>> [1]
>>>>
>>>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-316%3A+Introduce+SQL+Driver
>>>> <
>>>>
>>>
>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-316:+Introduce+SQL+Driver
>>>>>
>>>> [2] https://issues.apache.org/jira/browse/FLINK-26541 <
>>>> https://issues.apache.org/jira/browse/FLINK-26541>
>>>>
>>>> Best,
>>>> Paul Lam
>>>
>>
