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

feat: add standalone mode in java sdk #1485

Merged
merged 18 commits into from
Mar 30, 2022
Merged

Conversation

keyu813
Copy link
Contributor

@keyu813 keyu813 commented Mar 21, 2022

close #620
onebox/start_onebox.sh support standalone mode
close #1523

@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 2022

SDK Test Report

  70 files    70 suites   5m 56s ⏱️
171 tests 168 ✔️ 3 💤 0
210 runs  207 ✔️ 3 💤 0

Results for commit 89ea7ea.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 21, 2022

Linux Test Report

       52 files       179 suites   40m 22s ⏱️
  8 173 tests   8 173 ✔️ 0 💤 0
12 089 runs  12 089 ✔️ 0 💤 0

Results for commit 89ea7ea.

♻️ This comment has been updated with latest results.

@dl239
Copy link
Collaborator

dl239 commented Mar 21, 2022

add standalone test. start standalone openmldb in onebox/start_onebox.sh

@keyu813
Copy link
Contributor Author

keyu813 commented Mar 21, 2022

add standalone test. start standalone openmldb in onebox/start_onebox.sh

ok, should I add a StandaloneSmokeTest in /test ? or just update SQLRouterSmokeTest.java ?

@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #1485 (89ea7ea) into main (75f3c1b) will increase coverage by 0.10%.
The diff coverage is 95.65%.

@@             Coverage Diff              @@
##               main    #1485      +/-   ##
============================================
+ Coverage     66.51%   66.62%   +0.10%     
  Complexity      281      281              
============================================
  Files           583      587       +4     
  Lines        110438   111006     +568     
  Branches        974      981       +7     
============================================
+ Hits          73455    73953     +498     
- Misses        36791    36861      +70     
  Partials        192      192              
Impacted Files Coverage Δ
...in/java/com/_4paradigm/openmldb/sdk/SdkOption.java 88.88% <75.00%> (+9.94%) ⬆️
...paradigm/openmldb/sdk/impl/SqlClusterExecutor.java 52.85% <100.00%> (+2.35%) ⬆️
src/nameserver/name_server_impl.cc 37.98% <100.00%> (ø)
...paradigm/openmldb/batch/utils/ExpressionUtil.scala 45.13% <0.00%> (-8.64%) ⬇️
..._4paradigm/openmldb/batch/utils/HybridseUtil.scala 60.40% <0.00%> (-5.04%) ⬇️
hybridse/include/base/fe_status.h 92.30% <0.00%> (-3.70%) ⬇️
...ybridse/examples/toydb/src/tablet/tablet_catalog.h 63.15% <0.00%> (-3.51%) ⬇️
src/zk/dist_lock.cc 81.81% <0.00%> (-1.52%) ⬇️
src/catalog/tablet_catalog.cc 68.56% <0.00%> (-0.90%) ⬇️
src/catalog/base.cc 100.00% <0.00%> (ø)
... and 29 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 75f3c1b...89ea7ea. Read the comment docs.

“keyu813” added 2 commits March 30, 2022 10:56
onebox/start_onebox.sh Outdated Show resolved Hide resolved
onebox/start_onebox.sh Outdated Show resolved Hide resolved
sqlOpt.setEnable_debug(option.getEnableDebug());
sqlOpt.setRequest_timeout(option.getRequestTimeout());
this.sqlRouter = sql_router_sdk.NewClusterSQLRouter(sqlOpt);
sqlOpt.delete();
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't the GC handle it automatically ? @vagetablechicken

Copy link
Collaborator

@vagetablechicken vagetablechicken Mar 30, 2022

Choose a reason for hiding this comment

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

If swigCMemOwn==true, it'll gc. But we've found CMem cost too much or memory leak before(maybe gc is not immediate), so in java sdk, we manually delete CMem in every place.
Just leave it, until we can remove all manul deletion.

.github/workflows/sdk.yml Show resolved Hide resolved

static {
try {
SdkOption option = new SdkOption();
option.setZkPath(TestConfig.ZK_PATH);
option.setZkCluster(TestConfig.ZK_CLUSTER);
option.setSessionTimeout(200000);
router = new SqlClusterExecutor(option);
java.sql.Statement state = router.getStatement();
clusterRouter = new SqlClusterExecutor(option);
Copy link
Collaborator

Choose a reason for hiding this comment

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

must call Init() for both clusterRouter and standaloneRouter before use

Copy link
Contributor Author

Choose a reason for hiding this comment

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

must call Init() for both clusterRouter and standaloneRouter before use

which init()? SqlClusterExecutor seems have no Init() method

Copy link
Collaborator

@vagetablechicken vagetablechicken Mar 30, 2022

Choose a reason for hiding this comment

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

router->Init() is in SqlClusterExecutor. It's better to change the name. clusterRouter -> clusterExecutor or clusterClient

return isClusterMode;
}

public void setClusterMode(boolean clusterMode) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

where the option is set ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

where the option is set ?

before use SqlClusterExecutor we should set option, see SQLRouterSmokeTest

@aceforeverd aceforeverd merged commit caf3c3a into 4paradigm:main Mar 30, 2022
dl239 pushed a commit to dl239/OpenMLDB that referenced this pull request Mar 31, 2022
@lumianph lumianph mentioned this pull request Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PRE_AGG_META table can't create in standalone mode add standalone mode in java sdk
4 participants