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

[fix-9991][worker]fix statement is closed before resultSet.getMetaData() #10014

Merged
merged 6 commits into from
May 15, 2022
Merged

[fix-9991][worker]fix statement is closed before resultSet.getMetaData() #10014

merged 6 commits into from
May 15, 2022

Conversation

huangchenguang123
Copy link
Contributor

@huangchenguang123 huangchenguang123 commented May 12, 2022

fix statement is closed before resultSet.getMetaData(), fix #9991

@huangchenguang123 huangchenguang123 changed the title [fix][worker]fix statement is closed before resultSet.getMetaData() [fix-9991][worker]fix statement is closed before resultSet.getMetaData() May 12, 2022
@SbloodyS SbloodyS added first time contributor First-time contributor backend labels May 12, 2022
@mergeable mergeable bot removed first time contributor First-time contributor backend labels May 12, 2022
@SbloodyS SbloodyS added first time contributor First-time contributor backend labels May 12, 2022
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2022

Codecov Report

Merging #10014 (602f0ae) into dev (cbf0010) will increase coverage by 0.18%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                dev   #10014      +/-   ##
============================================
+ Coverage     40.68%   40.87%   +0.18%     
- Complexity     4579     4628      +49     
============================================
  Files           834      839       +5     
  Lines         33832    33983     +151     
  Branches       3745     3764      +19     
============================================
+ Hits          13766    13891     +125     
- Misses        18746    18762      +16     
- Partials       1320     1330      +10     
Impacted Files Coverage Δ
...er/master/dispatch/host/assign/RandomSelector.java 77.77% <0.00%> (-5.56%) ⬇️
...org/apache/dolphinscheduler/remote/utils/Host.java 37.77% <0.00%> (-2.23%) ⬇️
...dolphinscheduler/remote/future/ResponseFuture.java 81.96% <0.00%> (-1.64%) ⬇️
...e/dolphinscheduler/remote/NettyRemotingClient.java 52.11% <0.00%> (-1.41%) ⬇️
...r/plugin/task/sqoop/parameter/SqoopParameters.java 53.33% <0.00%> (-1.34%) ⬇️
...er/server/master/runner/WorkflowExecuteThread.java 8.02% <0.00%> (-0.23%) ⬇️
...nscheduler/service/process/ProcessServiceImpl.java 30.85% <0.00%> (ø)
...phinscheduler/plugin/task/jupyter/JupyterTask.java 65.47% <0.00%> (ø)
...eduler/plugin/task/jupyter/JupyterTaskChannel.java 0.00% <0.00%> (ø)
...cheduler/plugin/task/jupyter/JupyterConstants.java 0.00% <0.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbf0010...602f0ae. Read the comment docs.

Comment on lines 201 to 203
statement = prepareStatementAndBind(connection, mainStatementsBinds.get(0));
logger.info("main statement execute query, for sql: {}", mainStatementsBinds.get(0).getSql());
resultSet = statement.executeQuery();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not recommend this modification. You can put the resultProcess method in the query method, and the close method only has connection as an input parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its better to put the resultProcess method in the query method, i will fix my code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the close method only has connection as an input parameter, it is also recommended to modify.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resultSet is allways null, remove from the close method

huaangcg added 3 commits May 13, 2022 10:13
# Conflicts:
#	dolphinscheduler-task-plugin/dolphinscheduler-task-sql/src/main/java/org/apache/dolphinscheduler/plugin/task/sql/SqlTask.java
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 20 Code Smells

54.9% 54.9% Coverage
2.1% 2.1% Duplication

@huangchenguang123
Copy link
Contributor Author

huangchenguang123 commented May 14, 2022

@SbloodyS when i push my code and trigger check, sometimes the E2E/FileManage will fail, but i think my code won't effect the test case, I would like to know why this is causing this problem, this is my first contribute, so could you tell me the reasion?

@SbloodyS
Copy link
Member

@SbloodyS when i push my code and trigger check, sometimes the E2E/FileManage will fail, but i think my code won't effect the test case, I would like to know why this is causing this problem, this is my first contribute, so could you tell me the reasion?

I had restart the failed tests. You can try to trigger the retry by git push or closing and then reopening PR.

There may be some instability in E2E tests. I'll fix it in a few days.

@SbloodyS SbloodyS requested a review from zhuangchong May 14, 2022 10:47
Copy link
Contributor

@zhuangchong zhuangchong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@zhuangchong zhuangchong merged commit df04c4a into apache:dev May 15, 2022
@zhuangchong
Copy link
Contributor

@huangchenguang123
Thank you very much for your contribution, this PR is very good. I think we can get deep communiction, you can contact me through mail or add wechat(KerwinZhuang), when mail or added, please tell me who you are, I think I can help to familiar with the DolphinScheduler if you meet with problems.

@huangchenguang123 huangchenguang123 deleted the fix/fix_query branch May 15, 2022 15:13
zhongjiajie pushed a commit that referenced this pull request May 16, 2022
vagetablechicken added a commit to vagetablechicken/dolphinscheduler that referenced this pull request May 21, 2022
commit 8046180
Author: 王心雨 <wangxinyu@4paradigm.com>
Date:   Thu May 19 12:34:07 2022 +0800

    去掉错误提交的代码

commit 5e9e9a9
Author: 王心雨 <wangxinyu@4paradigm.com>
Date:   Thu May 19 12:23:44 2022 +0800

    加入openmldb task

commit b1885c7
Author: caishunfeng <caishunfeng2021@gmail.com>
Date:   Mon May 16 15:41:45 2022 +0800

    [Bug] fix run on docker and k8s (apache#10026)

    * fix docker-compose init schema

    * recovery depend on zk

    * update doc and dockerfile

    * fix run on k8s

    * udpate doc

    * add DOCKER flag & update doc

    * remove repeat DOCKER env

commit d643e1c
Author: Dannila <94423827+Dannila@users.noreply.github.com>
Date:   Mon May 16 15:06:21 2022 +0800

    [Fix-10039] Flink run command when perfecting Python jobs (apache#10042)

    * [fix] flink task

    * [fix] flink task

commit 3e471f8
Author: Jiajie Zhong <zhongjiajie955@hotmail.com>
Date:   Mon May 16 11:01:32 2022 +0800

    [feat] Add batch rerun and stop for process instance (apache#10013)

    * [feat] Add batch rerun and stop for process instance

    * fix execute type pass

    * error message changed

    * correct error message

    * add note to i18n ignore

    * enhance the note of process instance ids

commit f666c64
Author: chuxing <923622908@qq.com>
Date:   Mon May 16 09:53:45 2022 +0800

    [doc] Correct docs of development-environment-setup (apache#9995)

commit 359cbe2
Author: zixi0825 <sunzhaohe0825@gmail.com>
Date:   Sun May 15 17:46:31 2022 +0800

    [dataquality] Fix task commnd null bug (apache#9974)

commit 2423fa5
Author: xiangzihao <460888207@qq.com>
Date:   Sun May 15 13:04:48 2022 +0800

    [Feature-10034][CI] Add postgresql cluster test in CI (apache#10035)

    * add postgresql cluster test

    * add postgresql cluster test

    * add postgresql cluster test

    * add postgresql cluster test

commit df04c4a
Author: chuxing <923622908@qq.com>
Date:   Sun May 15 10:24:31 2022 +0800

    [fix-9991][worker]fix statement is closed before resultSet.getMetaData() (apache#10014)

commit baf654c
Author: xiangzihao <460888207@qq.com>
Date:   Sat May 14 12:30:57 2022 +0800

    [Feature-9474] [CI] Add cluster test script verify on shell script (apache#9997)

    * cluster test

    * fix init db failed

    * fix init db failed

    * fix init db failed

    * fix init db failed

    * fix init db failed

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add sudo

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * add github actions

    * remove cluster-test workflows

    * add github actions

    * add github actions

    * refactor test to docker mode

    * refactor test to docker mode

    * refactor test to docker mode

    * refactor test to docker mode

    * remove create schema logic

    * remove create schema logic

    * remove create schema logic

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * fix runnint cluster test error

    * add github actions

    * add github actions

    * add github actions

    * add cluster test timeout

    * add cluster start test loop check

    * add cluster start test loop check

    * optimize cluster start test loop check

commit ee2c516
Author: 旺阳 <wang@lqwang.net>
Date:   Fri May 13 10:39:05 2022 +0800

    [doc] Correct kubernetes (apache#9985)

    Co-authored-by: qingwli <qingwli@cisco.com>

commit d17379d
Author: rockfang <657297417@qq.com>
Date:   Fri May 13 10:16:54 2022 +0800

    [Fix][UI] Fix errorOutputPath column in dataquality page (apache#10015)

commit 8036936
Author: Eric Gao <ericgao.apache@gmail.com>
Date:   Thu May 12 19:55:49 2022 +0800

    [feat][task plugin] Add Jupyter task plugin (apache#9872)

    Co-authored-by: Amy0104 <97265214+Amy0104@users.noreply.github.com>

commit 0fe7548
Author: QuakeWang <45645138+QuakeWang@users.noreply.github.com>
Date:   Thu May 12 18:23:43 2022 +0800

    [doc] Add example and notice about task type Dependent (apache#10001)

    Co-authored-by: Jiajie Zhong <zhongjiajie955@gmail.com>

commit 18bfe63
Author: Amy0104 <97265214+Amy0104@users.noreply.github.com>
Date:   Thu May 12 16:54:03 2022 +0800

    [Fix][UI] Support only one file upload on the file manage page. (apache#10007)

commit dbdbfea
Author: WangJPLeo <103574007+WangJPLeo@users.noreply.github.com>
Date:   Thu May 12 16:31:53 2022 +0800

    [Fix-9975] The selected task instance was recreated when the Master service fail… (apache#9976)

    * The selected task instance was recreated when the Master service failed over.

    * Returns the expression result directly.

    * Use Recovery to determine whether to use the old task instance.

commit 00f1029
Author: Amy0104 <97265214+Amy0104@users.noreply.github.com>
Date:   Thu May 12 15:27:55 2022 +0800

    [Fix][UI] Fix the task name validator error. (apache#10008)

commit bce5a28
Author: Kerwin <37063904+zhuangchong@users.noreply.github.com>
Date:   Thu May 12 14:42:39 2022 +0800

    [doc] Add the description about execute type in SQL task (apache#9987)

commit 53cd700
Author: rockfang <657297417@qq.com>
Date:   Thu May 12 14:11:52 2022 +0800

    [Fix-10002] Fix some bugs in datasource list (apache#10005)

    * fix: fix ellipsis bug in table column

    * fix ellipsis bug in table column

    * fix: json-highlight component support nested jsonstring

    * fix: make datasource description show

commit 8d17fd2
Author: Jiajie Zhong <zhongjiajie955@hotmail.com>
Date:   Thu May 12 11:30:01 2022 +0800

    [ci] Dead link check all markdown file (apache#10004)

    there are 162 markdown files in dir `docs`
    and all file in project is 175 files, so I
    think check all file will not add too much
    load for our ci, but it could discovered
    the dead link in time to avoid some thing
    like apache#9998

commit 99b1c40
Author: songjianet <1778651752@qq.com>
Date:   Wed May 11 22:15:36 2022 +0800

    [Docs] Fix docker link. (apache#9998)

commit d4aeee1
Author: Tq <tianqitobethefirst@gmail.com>
Date:   Wed May 11 18:37:03 2022 +0800

    [Bug] [MASTER-9811]fix cmd param to overwrite global param when executing complement (apache#9952)

    * fix cmd param to overwrite global param when executing complement

    * fix cmd param to overwrite global param when executing complement

commit bc1c15b
Author: qianli2022 <97489722+qianli2022@users.noreply.github.com>
Date:   Wed May 11 16:40:30 2022 +0800

    [Feature-8252][doc] K8s and namespace manager docs and web page update (apache#9881)

    * test

    * test

    * doc

    * fix image error

    * fix

    * Update docs/docs/en/guide/security.md

    * Update docs/docs/en/guide/security.md

    * Update docs/docs/en/guide/security.md

    * Update docs/docs/zh/guide/security.md

    * fix doc

    Co-authored-by: qianl4 <qianl4@cicso.com>
    Co-authored-by: William Tong <weitong@cisco.com>
    Co-authored-by: Jiajie Zhong <zhongjiajie955@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend first time contributor First-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug-BE] [sql component] sql component failed to run
4 participants