Native cql changes #606

maheshrajamani · 2023-10-30T20:25:38Z

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

Changes manually tested
Automated Tests added/updated
Documentation added/updated
CLA Signed: DataStax CLA

into native_cql_changes

… native_cql_changes

into native_cql_changes

… native_cql_changes

jeffreyscarpenter

Lots of great work in here. My main concerns are:

We still have quite a bit of usage of bridge / gRPC proto classes. Should we create a followup ticket to complete this removal?
There appear to be quite a few tests that are still disabled. Which of these will be re-enabled before we merge this PR?

.github/workflows/continuous-integration.yaml

src/main/java/io/stargate/sgv2/jsonapi/config/OperationsConfig.java

src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableToErrorMapper.java

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/executor/CollectionSettings.java

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/executor/QueryExecutor.java

src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/CqlSessionCacheTest.java

jeffreyscarpenter · 2023-11-14T18:49:06Z

src/test/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/CountOperationTest.java

 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;

 @QuarkusTest
+@Disabled


Are we planning to re-enable this test at some point?

Need to figure out how to mock AsyncResultSet., then can be re-enabled.

jeffreyscarpenter · 2023-11-14T18:49:36Z

...ava/io/stargate/sgv2/jsonapi/service/operation/model/impl/CreateCollectionOperationTest.java

 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;

 @QuarkusTest
+@Disabled


When do we plan to re-enable?

jeffreyscarpenter · 2023-11-14T18:52:42Z

src/test/java/io/stargate/sgv2/jsonapi/service/schema/model/JsonapiTableMatcherTest.java

@@ -44,7 +65,7 @@ public void partitionColumnsTooMany() {
                  QueryOuterClass.ColumnSpec.newBuilder().setName("key2").build())
              .build();

-      boolean result = tableMatcher.test(table);
+      boolean result = tableMatcher.test(null);


hmm wondering why this would be null? here and other cases below. I do see this class is disabled.

@Hazel-Datastax ?

All the data structures in JsonapiTable have been changed and I have to rewrite all the unit tests. To save time, I didn't do that and just replaced the null to avoid the compile error. I will fix that later with all the unit tests.

@Hazel-Datastax Can you create an issue to fix this so can be taken at later time?

jeffreyscarpenter · 2023-11-14T18:55:10Z

src/test/java/io/stargate/sgv2/jsonapi/testresource/StargateTestResource.java

@@ -0,0 +1,264 @@
+//


This is temporary right? Are we planning to remove this class copied from main stargate repo prior to merging? (Presumably because Stargate version is updated with what we need)

Yes., Will remove this once the stargate api release is done.

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/executor/QueryExecutor.java

src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java

src/main/java/io/stargate/sgv2/jsonapi/config/OperationsConfig.java

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/executor/QueryExecutor.java

src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/TenantAwareCqlSessionBuilder.java

src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/CQLSessionCache.java

… native_cql_changes

tatu-at-datastax · 2023-11-15T16:19:56Z

src/main/java/io/stargate/sgv2/jsonapi/config/OperationsConfig.java

+    String localDatacenter();
+
+    /** Time to live for CQLSession in cache in seconds. */
+    @WithDefault("300")


Are we sure we really want 5 minutes over 1 -- this is not time-to-live for connection (AFAIK) but time unused connection remains for reuse after last call. That is, idle wait.

When session is evicted from cache, the session will be closed. Changed to 300 as per yesterday's conversation.

Also these values can be override using environment value in the chart.

… native_cql_changes

jeffreyscarpenter

Changes based on review feedback are complete and issues were created for follow up items, approving.

kathirsvn and others added 30 commits October 23, 2023 14:52

Query Executor interface for CQL

ad94017

[Native Protocol] Create and delete collection command changes (#581)

54ee2ec

Cache changes (#582)

02055dd

[Native Protocol]Changes for InsertOperation (#586)

f74b0e8

[Native Protocol] Read and count operation (#589)

40bc367

ReadAndUpdateOperation changes for native driver (#588)

3a858f5

Fix for empty filter

136fa76

Changes for delete command

67ccba4

Changes for keyspace commands

f9836a9

CQL Session changes

44a19c7

Fix for filter clauses

849e55c

Merge branch 'native_cql_changes' of https://github.com/stargate/jsonapi

d5bc2a0

into native_cql_changes

Logging added

4c49fa8

Fixed code for boolean sort

83694ad

Merge branch 'native_cql_changes' of https://github.com/stargate/jsonapi

82a6c2d

into native_cql_changes

CQL session fix

bea0649

Merge branch 'native_cql_changes' of github.com:stargate/jsonapi into…

2f46109

… native_cql_changes

CQL session fix

0832de8

Set default value to null so no need to serialize the key

a1abb8a

Merge branch 'native_cql_changes' of https://github.com/stargate/jsonapi

d39c199

into native_cql_changes

CQL session cache key fix

94d98f6

Merge branch 'native_cql_changes' of github.com:stargate/jsonapi into…

0575802

… native_cql_changes

CQL session cache key fix

92e0e85

IT bug fixes

b874e52

Avoid creating bridge Values

983420c

Moved cache configurations to quarkus properties

70944a2

CqlSessionCache key changes

bdbe639

Merge branch 'native_cql_changes' of github.com:stargate/jsonapi into…

d806343

… native_cql_changes

CQL session cache eviction listener added

e6090b5

getSessionCacheKey method improvement

873193c

Yuqi-Du and others added 3 commits November 13, 2023 17:32

IT passes. Finally!

c055b33

fix some unit tests

32be0e8

Native image fix related to Reflection

ac5cd48

maheshrajamani marked this pull request as ready for review November 14, 2023 18:29

maheshrajamani requested a review from a team as a code owner November 14, 2023 18:29

jeffreyscarpenter reviewed Nov 14, 2023

View reviewed changes

Yuqi-Du reviewed Nov 14, 2023

View reviewed changes

src/main/java/io/stargate/sgv2/jsonapi/service/bridge/executor/QueryExecutor.java Outdated Show resolved Hide resolved

Yuqi-Du reviewed Nov 14, 2023

View reviewed changes

src/main/java/io/stargate/sgv2/jsonapi/service/operation/model/impl/FindOperation.java Outdated Show resolved Hide resolved

Code refactored

61ae37a

maheshrajamani closed this Nov 14, 2023

maheshrajamani reopened this Nov 14, 2023

maheshrajamani commented Nov 14, 2023

View reviewed changes

maheshrajamani and others added 7 commits November 15, 2023 10:25

Removed CI to run against push for native_cql_changes branch

3d50b7b

Fixed executeRead consistency level

2c09a54

Merge branch 'native_cql_changes' of github.com:stargate/jsonapi into…

a657ef9

… native_cql_changes

SessionCache ttl session increased to 300.

c911532

Cleaned up TODO marks and removed secure connect bundle config

63e3a1e

Changed debug to trace logs in CQLSessionCache

9a6b2ef

Merge branch 'main' into native_cql_changes

bb4b4cd

tatu-at-datastax reviewed Nov 15, 2023

View reviewed changes

kathirsvn added 6 commits November 15, 2023 21:52

Fixed tests

0f5b017

Merge branch 'native_cql_changes' of github.com:stargate/jsonapi into…

62d3fbe

… native_cql_changes

Added JavaDoc to TenantAwareCqlSessionBuilder

10c3669

Changes to avoid consistency level conversions

a0ca6e0

bridge package moved to cqldriver

aa24af8

Formatting fix

e6d238d

jeffreyscarpenter approved these changes Nov 15, 2023

View reviewed changes

maheshrajamani merged commit 5113edc into main Nov 15, 2023
3 checks passed

maheshrajamani deleted the native_cql_changes branch November 15, 2023 18:21

jeffreyscarpenter mentioned this pull request Feb 14, 2024

Remove dependency for Stargate bridge - quarkus commons #673

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Native cql changes #606

Native cql changes #606

maheshrajamani commented Oct 30, 2023 •

edited

Loading

jeffreyscarpenter left a comment

jeffreyscarpenter Nov 14, 2023

maheshrajamani Nov 14, 2023

jeffreyscarpenter Nov 14, 2023

jeffreyscarpenter Nov 14, 2023

maheshrajamani Nov 14, 2023

Hazel-Datastax Nov 14, 2023

maheshrajamani Nov 15, 2023

Hazel-Datastax Nov 15, 2023

jeffreyscarpenter Nov 14, 2023

maheshrajamani Nov 14, 2023

tatu-at-datastax Nov 15, 2023

maheshrajamani Nov 15, 2023

maheshrajamani Nov 15, 2023

jeffreyscarpenter left a comment

Native cql changes #606

Native cql changes #606

Conversation

maheshrajamani commented Oct 30, 2023 • edited Loading

jeffreyscarpenter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeffreyscarpenter left a comment

Choose a reason for hiding this comment

maheshrajamani commented Oct 30, 2023 •

edited

Loading