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

Add ScyllaDB connector #10336

Closed
wants to merge 5 commits into from
Closed

Conversation

ebyhr
Copy link
Member

@ebyhr ebyhr commented Dec 17, 2021

Fixes #10276

@cla-bot cla-bot bot added the cla-signed label Dec 17, 2021
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from 505e424 to 04317ba Compare December 17, 2021 00:52
@ebyhr ebyhr added the enhancement New feature or request label Dec 17, 2021
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from bad8fe2 to 040e76c Compare December 17, 2021 02:46
@ebyhr ebyhr marked this pull request as ready for review December 17, 2021 08:04
@findepi
Copy link
Member

findepi commented Dec 17, 2021

cc @haaawk

@ebyhr
Copy link
Member Author

ebyhr commented Dec 22, 2021

@robd003 Can you test this PR with your ScyllaDB cluster?

@robd003
Copy link

robd003 commented Dec 22, 2021

Yes! Traveling today but will push the testEnvironment for Scylla and test this PR.

@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from db379aa to dded82f Compare December 23, 2021 00:46
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from 0e4c015 to 66c373e Compare February 25, 2022 02:48
@github-actions github-actions bot added the docs label Feb 25, 2022
@ebyhr ebyhr requested a review from hashhar February 25, 2022 05:40
@ebyhr ebyhr marked this pull request as draft February 26, 2022 01:30
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from 5b22745 to dda5aaf Compare March 3, 2022 03:01
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from dda5aaf to 76982e3 Compare May 6, 2022 00:04
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from 76982e3 to c658433 Compare May 6, 2022 01:09
@ebyhr ebyhr marked this pull request as ready for review May 10, 2022 06:43
Copy link
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

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

Looks good to me, mostly test changes and new tests.

<ignoredDependencies>
<dependency>
<groupId>com.datastax.oss</groupId>
<artifactId>java-driver-core</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Which dependency does it get duplicated by?

@hashhar
Copy link
Member

hashhar commented May 15, 2022

Please rebase.

@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from c658433 to a13ee1c Compare May 16, 2022 00:40
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from a13ee1c to 9579cd5 Compare May 16, 2022 03:10
@findinpath
Copy link
Contributor

Is there any reason for keeping the following commits separate :

?

@ebyhr
Copy link
Member Author

ebyhr commented May 16, 2022

@findinpath To keep clean diff.

@findinpath
Copy link
Contributor

I get locally while trying to run:

testing/bin/ptl test  run --environment multinode-scylla -- -t  io.trino.tests.product.scylla.TestScylla

the following exception:

2022-05-16 10:24:25 INFO: Using native clock for microsecond precision
tests               | java.lang.RuntimeException: error invoking methods annotated with io.trino.tempto.BeforeTestWithContext
tests               | 	at io.trino.tempto.internal.initialization.TestInitializationListener.invokeMethodsAnnotatedWith(TestInitializationListener.java:265)
tests               | 	at io.trino.tempto.internal.initialization.TestInitializationListener.runBeforeWithContextMethods(TestInitializationListener.java:243)
tests               | 	at io.trino.tempto.internal.initialization.TestInitializationListener.onTestStart(TestInitializationListener.java:200)
tests               | 	at org.testng.internal.Invoker.runTestListeners(Invoker.java:1724)
tests               | 	at org.testng.internal.Invoker.runTestListeners(Invoker.java:1699)
tests               | 	at org.testng.internal.Invoker.invokeMethod(Invoker.java:622)
tests               | 	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
tests               | 	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
tests               | 	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
tests               | 	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
tests               | 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
tests               | 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
tests               | 	at java.base/java.lang.Thread.run(Thread.java:829)
tests               | Caused by: java.lang.reflect.InvocationTargetException
tests               | 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
tests               | 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
tests               | 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
tests               | 	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
tests               | 	at io.trino.tempto.internal.initialization.TestInitializationListener.invokeMethodsAnnotatedWith(TestInitializationListener.java:262)
tests               | 	... 12 more
tests               | Caused by: java.lang.IllegalStateException: Since you provided explicit contact points, the local DC must be explicitly set (see basic.load-balancing-policy.local-datacenter in the config, or set it programmatically with SessionBuilder.withLocalDatacenter). Current contact points are: Node(endPoint=scylla/172.19.0.2:9042, hostId=6dcdd0ad-5fac-403d-ac54-ae2c3b0d38e5, hashCode=39fb7c7f)=datacenter1. Current DCs in this cluster are: datacenter1

@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch 2 times, most recently from e88a79e to c039442 Compare May 17, 2022 08:15
@findinpath findinpath self-requested a review May 17, 2022 10:32
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from c039442 to 7820f4c Compare May 28, 2022 00:17
ebyhr added 5 commits May 28, 2022 09:18
This commit is preparation for the next commit.
Set minimum required version as 3.0.0 because
ScyllaDB version 2.x can't detect partition
correctly and it leads to query failure.
@ebyhr ebyhr force-pushed the ebi/scylladb-connector branch from 7820f4c to bf0365f Compare May 28, 2022 00:18
@ebyhr
Copy link
Member Author

ebyhr commented Jul 6, 2022

@robd003 Gentler reminder.

@ebyhr
Copy link
Member Author

ebyhr commented Aug 3, 2022

I will reopen if necessary.

@ebyhr ebyhr closed this Aug 3, 2022
@ebyhr ebyhr deleted the ebi/scylladb-connector branch August 3, 2022 02:50
@mkorolyov
Copy link

@ebyhr Hey sir, just wondering why this wonderful PR was closed? what could i do to merge scylladb connector?

@ebyhr
Copy link
Member Author

ebyhr commented Jun 28, 2023

I closed this PR as the requested contributor ghosted. I will reopen this PR if many people are interested in this connector.

@danielhe4rt
Copy link

Hey! Anything that we can do to make this PR merged? I work at ScyllaDB, maybe we can help you guys in some way to make it happen.

Cheers!

@ebyhr
Copy link
Member Author

ebyhr commented Jun 28, 2023

Reopened the PR in #18074

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed docs enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Dedicated ScyllaDB connector
7 participants