Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
frankgh merged PR #59: URL: https://github.com/apache/cassandra-analytics/pull/59 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
yifan-c commented on code in PR #59:
URL:
https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597650872
##
cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java:
##
@@ -486,20 +496,23 @@ public static Cluster createDriverCluster(ICluster dtest)
static class IntegrationTestModule extends AbstractModule
{
private static final Logger LOGGER =
LoggerFactory.getLogger(IntegrationTestModule.class);
-private final ICluster cluster;
+private final Iterable instances;
Review Comment:
Now I see
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
yifan-c commented on code in PR #59: URL: https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597648858 ## cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java: ## @@ -428,9 +436,11 @@ protected Object[][] queryAllData(QualifiedName table) * @param consistencyLevel the consistency level to use for querying the data * @return all the data queried from the table */ -protected Object[][] queryAllData(QualifiedName table, ConsistencyLevel consistencyLevel) +protected Object[][] queryAllData(QualifiedName table, String consistencyLevel) Review Comment: yep. "consolidate into a single CL type" sounds good and makes it easier to maintain in the future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
frankgh commented on code in PR #59:
URL:
https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597633049
##
cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java:
##
@@ -486,20 +496,23 @@ public static Cluster createDriverCluster(ICluster dtest)
static class IntegrationTestModule extends AbstractModule
{
private static final Logger LOGGER =
LoggerFactory.getLogger(IntegrationTestModule.class);
-private final ICluster cluster;
+private final Iterable instances;
Review Comment:
The goal is to allow a Sidecar service to handle a subset of instances from
the cluster. For that we don't expose the cluster itself, but rather only give
the Sidecar module access to the instances it has to handle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
frankgh commented on code in PR #59: URL: https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597632737 ## cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java: ## @@ -428,9 +436,11 @@ protected Object[][] queryAllData(QualifiedName table) * @param consistencyLevel the consistency level to use for querying the data * @return all the data queried from the table */ -protected Object[][] queryAllData(QualifiedName table, ConsistencyLevel consistencyLevel) +protected Object[][] queryAllData(QualifiedName table, String consistencyLevel) Review Comment: The main reason to use String here is convenience. At `org.apache.cassandra.sidecar.testing.SharedClusterIntegrationTestBase` we use type `org.apache.cassandra.distributed.api.ConsistencyLevel`, but at the call sites we have other types available. In this case, we have type `com.datastax.driver.core.ConsistencyLevel` from the `org.apache.cassandra.analytics.TestConsistencyLevel`. Alternatively we can try to consolidate into a single CL type that we use across all of the tests. WDYT? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
frankgh commented on code in PR #59:
URL:
https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597630564
##
cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java:
##
@@ -486,20 +496,23 @@ public static Cluster createDriverCluster(ICluster dtest)
static class IntegrationTestModule extends AbstractModule
{
private static final Logger LOGGER =
LoggerFactory.getLogger(IntegrationTestModule.class);
-private final ICluster cluster;
+private final Iterable instances;
Review Comment:
This is so we can customize the number of instances handled by Sidecar like
we do here:
https://github.com/apache/cassandra-analytics/pull/59/files#diff-a23a3d0a797ed790cbf779edbb5f6892cb31acff2d2f9bdc9c10a3347411b6d9R103
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19634: Improve test coverage for downed instances [cassandra-analytics]
yifan-c commented on code in PR #59:
URL:
https://github.com/apache/cassandra-analytics/pull/59#discussion_r1597549593
##
cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java:
##
@@ -428,9 +436,11 @@ protected Object[][] queryAllData(QualifiedName table)
* @param consistencyLevel the consistency level to use for querying the
data
* @return all the data queried from the table
*/
-protected Object[][] queryAllData(QualifiedName table, ConsistencyLevel
consistencyLevel)
+protected Object[][] queryAllData(QualifiedName table, String
consistencyLevel)
Review Comment:
nit: prefer type over string
##
cassandra-analytics-integration-framework/src/main/java/org/apache/cassandra/sidecar/testing/SharedClusterIntegrationTestBase.java:
##
@@ -486,20 +496,23 @@ public static Cluster createDriverCluster(ICluster dtest)
static class IntegrationTestModule extends AbstractModule
{
private static final Logger LOGGER =
LoggerFactory.getLogger(IntegrationTestModule.class);
-private final ICluster cluster;
+private final Iterable instances;
Review Comment:
`ICluster` is already the `Iterable`. Why changing to the new type here?
Beside, `ICluster` has convenient methods to return a stream of the
instances.
##
cassandra-analytics-integration-tests/src/test/java/org/apache/cassandra/analytics/BulkWriteDownInstanceTest.java:
##
@@ -0,0 +1,145 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.analytics;
+
+import java.util.AbstractMap;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import org.apache.cassandra.distributed.api.IInstance;
+import org.apache.cassandra.sidecar.testing.QualifiedName;
+import org.apache.cassandra.spark.bulkwriter.WriterOptions;
+import org.apache.cassandra.testing.ClusterBuilderConfiguration;
+import org.apache.spark.sql.DataFrameWriter;
+import org.apache.spark.sql.Dataset;
+import org.apache.spark.sql.Row;
+import org.apache.spark.sql.SparkSession;
+
+import static com.datastax.driver.core.ConsistencyLevel.ALL;
+import static com.datastax.driver.core.ConsistencyLevel.LOCAL_QUORUM;
+import static com.datastax.driver.core.ConsistencyLevel.ONE;
+import static
org.apache.cassandra.analytics.ResiliencyTestBase.uniqueTestTableFullName;
+import static
org.apache.cassandra.testing.TestUtils.CREATE_TEST_TABLE_STATEMENT;
+import static org.apache.cassandra.testing.TestUtils.DC1_RF3;
+import static org.apache.cassandra.testing.TestUtils.ROW_COUNT;
+import static org.apache.cassandra.testing.TestUtils.TEST_KEYSPACE;
+
+/**
+ * Tests bulk writes in different scenarios when Cassandra instances are down
+ */
+class BulkWriteDownInstanceTest extends SharedClusterSparkIntegrationTestBase
+{
+Set downInstances = new HashSet<>();
+
+@ParameterizedTest(name = "{index} => instanceDownCount={0} {1}")
Review Comment:
nit: so it outputs `instanceDownCount=0, readCL=ONE, writeCL=ALL`
```suggestion
@ParameterizedTest(name = "{index} => instanceDownCount={0}, {1}")
```
##
cassandra-analytics-integration-tests/src/test/java/org/apache/cassandra/analytics/replacement/HostReplacementMultiDCInsufficientReplicasTest.java:
##
@@ -62,25 +61,10 @@ class HostReplacementMultiDCInsufficientReplicasTest
extends HostReplacementTest
@Test
void nodeReplacementFailureMultiDCInsufficientNodes()
{
-Throwable thrown = catchThrowable(() ->
- bulkWriterDataFrameWriter(df,
QUALIFIED_NAME)
-
.option(WriterOptions.BULK_WRITER_CL.name(), EACH_QUORUM.name())
- .save());
+DataFrameWriter dfWriter = bulkWriterDataFrameWriter(df,
QUALIFIED_NAME)
+
Re: [PR] CASSANDRA-19626 Fix NullPointerException when reading static column w… [cassandra-analytics]
frankgh merged PR #58: URL: https://github.com/apache/cassandra-analytics/pull/58 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
ajweave commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1596024998
##
core/src/main/java/com/datastax/oss/driver/api/core/tracker/RequestTracker.java:
##
@@ -52,20 +68,23 @@ default void onSuccess(
* @param executionProfile the execution profile of this request.
* @param node the node that returned the successful response.
* @param requestLogPrefix the dedicated log prefix for this request
+ * @param executionInfo the execution info containing the results of this
request
*/
default void onSuccess(
@NonNull Request request,
long latencyNanos,
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
- @NonNull String requestLogPrefix) {
-// If client doesn't override onSuccess with requestLogPrefix delegate
call to the old method
-onSuccess(request, latencyNanos, executionProfile, node);
+ @NonNull String requestLogPrefix,
Review Comment:
Refactored to make requestLogPrefix the last arg in all the methods.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
ajweave commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1596015592
##
core/src/main/java/com/datastax/oss/driver/api/core/tracker/RequestTracker.java:
##
@@ -148,16 +204,17 @@ default void onNodeSuccess(
* @param executionProfile the execution profile of this request.
* @param node the node that returned the successful response.
* @param requestLogPrefix the dedicated log prefix for this request
+ * @param executionInfo the execution info containing the results of this
request
*/
default void onNodeSuccess(
@NonNull Request request,
long latencyNanos,
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
- @NonNull String requestLogPrefix) {
-// If client doesn't override onNodeSuccess with requestLogPrefix delegate
call to the old
-// method
-onNodeSuccess(request, latencyNanos, executionProfile, node);
+ @NonNull String requestLogPrefix,
+ @NonNull ExecutionInfo executionInfo) {
+// delegate call to the old method
+onNodeSuccess(request, latencyNanos, executionProfile, node,
requestLogPrefix);
Review Comment:
Changes to `ContinuousRequestHandlerBase` merged. Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed [cassandra-java-driver]
absurdfarce commented on PR #1929: URL: https://github.com/apache/cassandra-java-driver/pull/1929#issuecomment-2103041959 This functionality has been implemented in #1932 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed [cassandra-java-driver]
absurdfarce closed pull request #1929: CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed URL: https://github.com/apache/cassandra-java-driver/pull/1929 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
adutra commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1595217254
##
core/src/main/java/com/datastax/oss/driver/api/core/tracker/RequestTracker.java:
##
@@ -52,20 +68,23 @@ default void onSuccess(
* @param executionProfile the execution profile of this request.
* @param node the node that returned the successful response.
* @param requestLogPrefix the dedicated log prefix for this request
+ * @param executionInfo the execution info containing the results of this
request
*/
default void onSuccess(
@NonNull Request request,
long latencyNanos,
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
- @NonNull String requestLogPrefix) {
-// If client doesn't override onSuccess with requestLogPrefix delegate
call to the old method
-onSuccess(request, latencyNanos, executionProfile, node);
+ @NonNull String requestLogPrefix,
Review Comment:
Small request: would it be possible to keep the log prefix as the _last_
parameter? This has been kind of a tacit convention so far.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19568: Use Jabba to specify Java 1.8 for building the driver [cassandra-java-driver]
absurdfarce merged PR #1932: URL: https://github.com/apache/cassandra-java-driver/pull/1932 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
nitinitt commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2101687257 Thanks @absurdfarce! I have addressed the 2 minor documentation comments as part of: https://github.com/apache/cassandra-java-driver/pull/1933/files -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] JAVA-3142: Improving the documentation for remote local dc's feature [cassandra-java-driver]
nitinitt opened a new pull request, #1933: URL: https://github.com/apache/cassandra-java-driver/pull/1933 This PR is to address code review comment in merged [PR](https://github.com/apache/cassandra-java-driver/pull/1896) - https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1585228162 - https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1585228162 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] CASSANDRA-19626 Fix NullPointerException when reading static column w… [cassandra-analytics]
frankgh opened a new pull request, #58: URL: https://github.com/apache/cassandra-analytics/pull/58 …ith null values Patch by Francisco Guerrero; Reviewed by Yifan Cai for CASSANDRA-19626 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
ajweave commented on PR #1640: URL: https://github.com/apache/cassandra-java-driver/pull/1640#issuecomment-2101654238 Sorry for the delay. I will get this done tomorrow. I appreciate the attention here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
SiyaoIsHiding commented on PR #1640: URL: https://github.com/apache/cassandra-java-driver/pull/1640#issuecomment-2101624573 May you please merge apache/4.x to your branch? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
SiyaoIsHiding commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1594781483
##
core/src/main/java/com/datastax/dse/driver/internal/core/cql/continuous/ContinuousRequestHandlerBase.java:
##
@@ -1590,18 +1597,20 @@ private void completeResultSetFuture(
private ExecutionInfo createExecutionInfo(@NonNull Result result,
@Nullable Frame response) {
ByteBuffer pagingState =
result instanceof Rows ? ((Rows) result).getMetadata().pagingState :
null;
- return new DefaultExecutionInfo(
- statement,
- node,
- startedSpeculativeExecutionsCount.get(),
- executionIndex,
- errors,
- pagingState,
- response,
- true,
- session,
- context,
- executionProfile);
+ this.executionInfo =
Review Comment:
It's a bit of work to set up. I ran all the tests against this branch, and
it passed all the tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] [CASSANDRA-19625] Initial Configuration for SonarCube Analysis [cassandra-analytics]
5 opened a new pull request, #57: URL: https://github.com/apache/cassandra-analytics/pull/57 Introduce the initial Gradle configuration for integration of Cassandra Analytics with SonarCube for code analysis -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] An experiment [cassandra-java-driver]
absurdfarce opened a new pull request, #1932: URL: https://github.com/apache/cassandra-java-driver/pull/1932 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce merged PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2101007620 Thanks @nitinitt, this looks great! I concur that this commit looks like it contains all the fixes we've been discussing so we are ready to get this guy in! Big thank you for all your help in making this happen! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce commented on code in PR #1896:
URL:
https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1585228162
##
core/src/main/resources/reference.conf:
##
@@ -574,6 +574,11 @@ datastax-java-driver {
# Modifiable at runtime: no
# Overridable in a profile: yes
allow-for-local-consistency-levels = false
+ # Ordered preference list of remote dc's (in order) optionally supplied
for automatic failover. While building a query plan, the driver uses the DC's
supplied in order together with max-nodes-per-remote-dc
Review Comment:
Another nit: might be worth mentioning that users aren't required to specify
all DCs when listing preferences via this config.
##
core/src/main/resources/reference.conf:
##
@@ -574,6 +574,11 @@ datastax-java-driver {
# Modifiable at runtime: no
# Overridable in a profile: yes
allow-for-local-consistency-levels = false
+ # Ordered preference list of remote dc's (in order) optionally supplied
for automatic failover. While building a query plan, the driver uses the DC's
supplied in order together with max-nodes-per-remote-dc
Review Comment:
Nit: should be a space between this and allow-for-local-consistency-levels
in order to make it clear we're talking about two distinct configs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3118: Add support for vector data type in Schema Builder, QueryBuilder [cassandra-java-driver]
michaelsembwever commented on PR #1931: URL: https://github.com/apache/cassandra-java-driver/pull/1931#issuecomment-2100603793 what's the CASSANDRA ticket for this ? i will test this downstream here: - https://github.com/spring-projects/spring-ai/blob/main/vector-stores/spring-ai-cassandra/src/main/java/org/springframework/ai/vectorstore/CassandraVectorStoreConfig.java#L551-L568 - https://github.com/spring-projects/spring-ai/blob/main/vector-stores/spring-ai-cassandra/src/main/java/org/springframework/ai/vectorstore/CassandraVectorStoreConfig.java#L502-L512 - https://github.com/spring-projects/spring-ai/blob/main/vector-stores/spring-ai-cassandra/src/main/java/org/springframework/ai/vectorstore/CassandraVectorStore.java#L342-L345 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
nitinitt commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2099492321 Thanks @absurdfarce for the guidance and the patience. I have squashed and force pushed and here is how git log looks to e: ``` Author: Nitin Chhabra Date: Thu Nov 30 12:38:23 2023 -0800 JAVA-3142: Ability to specify ordering of remote local dc's via new configuration for graceful automatic failovers patch by Nitin Chhabra; reviewed by Alexandre Dutra, Andy Tolbert, and Bret McGuire for JAVA-3142 ``` Sorry was struggling with formatting in the git message little bit hence you see extra force push. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3051: Memory leak [cassandra-java-driver]
SiyaoIsHiding commented on code in PR #1743:
URL:
https://github.com/apache/cassandra-java-driver/pull/1743#discussion_r1593183103
##
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java:
##
@@ -96,14 +99,38 @@ public class DefaultLoadBalancingPolicy extends
BasicLoadBalancingPolicy impleme
private static final int MAX_IN_FLIGHT_THRESHOLD = 10;
private static final long RESPONSE_COUNT_RESET_INTERVAL_NANOS =
MILLISECONDS.toNanos(200);
- protected final Map responseTimes = new
ConcurrentHashMap<>();
+ protected final LoadingCache responseTimes;
protected final Map upTimes = new ConcurrentHashMap<>();
private final boolean avoidSlowReplicas;
public DefaultLoadBalancingPolicy(@NonNull DriverContext context, @NonNull
String profileName) {
super(context, profileName);
this.avoidSlowReplicas =
profile.getBoolean(DefaultDriverOption.LOAD_BALANCING_POLICY_SLOW_AVOIDANCE,
true);
+CacheLoader cacheLoader =
+new CacheLoader() {
+ @Override
+ public AtomicLongArray load(Node key) throws Exception {
+// The array stores at most two timestamps, since we don't need
more;
+// the first one is always the least recent one, and hence the one
to inspect.
+long now = nanoTime();
+AtomicLongArray array = responseTimes.getIfPresent(key);
+if (array == null) {
+ array = new AtomicLongArray(1);
+ array.set(0, now);
+} else if (array.length() == 1) {
+ long previous = array.get(0);
+ array = new AtomicLongArray(2);
+ array.set(0, previous);
+ array.set(1, now);
+} else {
+ array.set(0, array.get(1));
+ array.set(1, now);
+}
+return array;
+ }
+};
+this.responseTimes =
CacheBuilder.newBuilder().weakKeys().build(cacheLoader);
Review Comment:
@aratno Hi Abe, thank you for your review. Is there any update?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2099192505 Excellent, that looks great @nitinitt! Thanks for jumping on that so quickly! As for your question: we do indeed plan on merging this ticket before the next release (version 4.18.1). In fact, according to [our planning document](https://docs.google.com/document/d/1MM7RAbcWTmjURCEXcWfAPuMNVIr-0zXkLXy6H-G82Xs/edit#heading=h.z0c6bi62ifoh), this PR is the _last_ item we planned to get in for 4.18.1. So once we're done here we should be ready to go. The last thing we need to do is to squash all the commits on this feature branch down to a single commit. You can do that with git via an interactive rebase. There's a decent article that walks through the steps [here](https://www.baeldung.com/ops/git-squash-commits); you're specifically interested in [this section](https://www.baeldung.com/ops/git-squash-commits#1-squash-the-last-x-commits). Once you've successfully squashed the commits you'll need to do a force push to your repository; you're actually re-writing your git history via this process so a force push is required. In your case the commands should look something like the following: ``` git rebase -i HEAD~9 git log # To confirm your history has been re-written git push origin –force ``` I've run this rebase operation locally a few times now against 4.x and I can confirm the patch applies cleanly. You may also want to consider saving a patch for the current (i.e. final) state of this branch so that we can re-create it if necessary. You can do that with the following command (similar to what you used before): ``` curl -L https://github.com/apache/cassandra-java-driver/pull/1896.patch > 1896-final.patch ``` I've also done this so I also have a copy of all the changes on this feature branch. Finally, regarding the commit message of your squashed commit; this should look something like the following: ``` JAVA-3142: Ability to specify ordering of remote local dc's via new configuration for graceful automatic failovers patch by Nitin Chhabra; reviewed by Alexandre Dutra, Andy Tolbert, and Bret McGuire for JAVA-3142 ``` If you have any questions about any of the above comments please don't hesitate to ask! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
nitinitt commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2098926296 > @nitinitt Probably the most straightforward way to incorporate the changes would be to get a patch file from the other PR and then apply that patch to this PR. You can get the patch file with something like the following command: > > ``` > curl -L https://github.com/absurdfarce/cassandra-java-driver/pull/1.patch > 1896-refactor.patch > ``` > > You can then apply the changes using git. If you want to just bring in the changes and commit them yourself you can use "git apply": > > ``` > git apply 1896-refactor.patch > ``` > > Or you can directly create new commits in your repo for every commit in the patch file using "git am": > > ``` > git am 1896-refactor.patch > ``` > > If you're at all unsure about what to do it's probably best to start with "git apply". The changes will be applied to the files but nothing will be committed; you can always do a "git reset" to get your repo back to the state of your last commit. Note that if you have any other local changes "git reset" will remove those as well so be careful when you use it! > > I just tested this locally and a patch file built of of commit [this commit](https://github.com/absurdfarce/cassandra-java-driver/pull/1/commits/0e010cb5d2bc8697a47cad7f3ce39aaefdb5fe92) applies cleanly to a working branch containing the same commits you have in your PR branch. Thank you @absurdfarce for the steps, I was able to get the patch and apply it over the last commit using: `git apply 1896-refactor.patch`. Would this PR be merged and part of next release? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2098596820 @nitinitt Probably the most straightforward way to incorporate the changes would be to get a patch file from the other PR and then apply that patch to this PR. You can get the patch file with something like the following command: ``` curl -L https://github.com/absurdfarce/cassandra-java-driver/pull/1.patch > 1896-refactor.patch ``` You can then apply the changes using git. If you want to just bring in the changes and commit them yourself you can use "git apply": ``` git apply 1896-refactor.patch ``` Or you can directly create new commits in your repo for every commit in the patch file using "git am": ``` git am 1896-refactor.patch ``` If you're at all unsure about what to do it's probably best to start with "git apply". The changes will be applied to the files but nothing will be committed; you can always do a "git reset" to get your repo back to the state of your last commit. Note that if you have any other local changes "git reset" will remove those as well so be careful when you use it! I just tested this locally and a patch file built of of commit 0e010cb5d2bc8697a47cad7f3ce39aaefdb5fe92 applies cleanly to a working branch containing the same commits you have in your PR branch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
nitinitt commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2097545011 Thanks @tolbertam and @absurdfarce for taking the time and helping to simplify the code with [refactored PR](https://github.com/absurdfarce/cassandra-java-driver/pull/1 ). I am not sure how the changes would be integrated, or should I sync this PR with branch: https://github.com/absurdfarce/cassandra-java-driver/tree/java3142-refactor -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
absurdfarce commented on PR #1896: URL: https://github.com/apache/cassandra-java-driver/pull/1896#issuecomment-2097402276 Apologies @nitinitt , I know this has taken a long time to get to... I really appreciate your patience. Overall I'm pretty positive on the change but I'm a bit concerned about one aspect of the implementation. As currently written the changes intermingle the "preferred DC" and "no preferred DC" cases when (maybe) adding remote DC failover nodes. I'd rather make that division much cleaner so that it's very clear which part of the code does what. I've taken a stab at [a set of changes on top of this PR](https://github.com/absurdfarce/cassandra-java-driver/pull/1) (as of this writing) to show that what might look like. Would you mind taking a look and letting me know what you think? Thanks again! And thanks to @adutra and @tolbertam for their reviews as well! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
tolbertam commented on code in PR #1896:
URL:
https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1591774161
##
core/src/main/java/com/datastax/oss/driver/api/core/config/TypedDriverOption.java:
##
@@ -892,6 +892,16 @@ public String toString() {
DefaultDriverOption.LOAD_BALANCING_DC_FAILOVER_ALLOW_FOR_LOCAL_CONSISTENCY_LEVELS,
GenericType.BOOLEAN);
+ /**
+ * Ordered preference list of remote dc's optionally supplied for automatic
failover and included
Review Comment:
```suggestion
* Ordered preference list of remote dcs optionally supplied for automatic
failover and included
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3142: Ability to optionally specify remote dcs for deterministic failovers when remote dcs are used in query plan [cassandra-java-driver]
tolbertam commented on code in PR #1896:
URL:
https://github.com/apache/cassandra-java-driver/pull/1896#discussion_r1591773768
##
core/src/main/java/com/datastax/oss/driver/api/core/config/DefaultDriverOption.java:
##
@@ -982,7 +982,13 @@ public enum DefaultDriverOption implements DriverOption {
* Value-type: {@link java.time.Duration}
*/
SSL_KEYSTORE_RELOAD_INTERVAL("advanced.ssl-engine-factory.keystore-reload-interval"),
- ;
+ /**
+ * Ordered preference list of remote dc's optionally supplied for automatic
failover.
Review Comment:
Tiny suggestion:
```suggestion
* Ordered preference list of remote dcs optionally supplied for automatic
failover.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] JAVA-3118: Add support for vector data type in Schema Builder, QueryBuilder [cassandra-java-driver]
SiyaoIsHiding opened a new pull request, #1931: URL: https://github.com/apache/cassandra-java-driver/pull/1931 The current PR is only about the `ANN OF` in the `QueryBuiler`, not anything else yet. I would appreciate it if this PR undergo an initial review, which will help my future development in `SchemaBuilder`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19616: Integrate with the latest sidecar client [cassandra-analytics]
yifan-c merged PR #56: URL: https://github.com/apache/cassandra-analytics/pull/56 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] CASSANDRA-19616: Integrate with the latest sidecar client [cassandra-analytics]
yifan-c opened a new pull request, #56: URL: https://github.com/apache/cassandra-analytics/pull/56 The patch updates the analytics code to consume the latest sidecar client after CASSANDRASC-127 Patch by Yifan Cai; Reviewed by Francisco Guerrero for CASSANDRA-19616 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
SiyaoIsHiding commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1587797662
##
core/src/main/java/com/datastax/oss/driver/api/core/tracker/RequestTracker.java:
##
@@ -148,16 +204,17 @@ default void onNodeSuccess(
* @param executionProfile the execution profile of this request.
* @param node the node that returned the successful response.
* @param requestLogPrefix the dedicated log prefix for this request
+ * @param executionInfo the execution info containing the results of this
request
*/
default void onNodeSuccess(
@NonNull Request request,
long latencyNanos,
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
- @NonNull String requestLogPrefix) {
-// If client doesn't override onNodeSuccess with requestLogPrefix delegate
call to the old
-// method
-onNodeSuccess(request, latencyNanos, executionProfile, node);
+ @NonNull String requestLogPrefix,
+ @NonNull ExecutionInfo executionInfo) {
+// delegate call to the old method
+onNodeSuccess(request, latencyNanos, executionProfile, node,
requestLogPrefix);
Review Comment:
I also explored how to pass in ExecutionInfo from
ContinuousRequestHandlerBase, see
[here](https://github.com/SiyaoIsHiding/java-driver/compare/4.x...SiyaoIsHiding:java-driver:request-tracker-execution-info?expand=1).
How do you think of this approach?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19451: An Option of Latency Sensitive Load Balancing Policy [cassandra-java-driver]
absurdfarce commented on code in PR #1915: URL: https://github.com/apache/cassandra-java-driver/pull/1915#discussion_r1587128032 ## core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/LatencySensitiveLoadBalancingPolicy.java: ## @@ -0,0 +1,275 @@ +/* Review Comment: I still need to give this a more complete review but at a minimum this class needs to go in the [examples directory](https://github.com/apache/cassandra-java-driver/tree/4.x/examples/src/main/java/com/datastax/oss/driver/examples) rather than inline with the default LBP like this. In 4.x the default LBP is intended to be the primary LBP used in nearly all situations; adding another LBP into the same package will significantly confuse that messaging. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3129: Change Snappy version and corresponding manual page [cassandra-java-driver]
SiyaoIsHiding closed pull request #1736: JAVA-3129: Change Snappy version and corresponding manual page URL: https://github.com/apache/cassandra-java-driver/pull/1736 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
SiyaoIsHiding commented on code in PR #1640:
URL:
https://github.com/apache/cassandra-java-driver/pull/1640#discussion_r1586980425
##
core/src/main/java/com/datastax/oss/driver/api/core/tracker/RequestTracker.java:
##
@@ -148,16 +204,17 @@ default void onNodeSuccess(
* @param executionProfile the execution profile of this request.
* @param node the node that returned the successful response.
* @param requestLogPrefix the dedicated log prefix for this request
+ * @param executionInfo the execution info containing the results of this
request
*/
default void onNodeSuccess(
@NonNull Request request,
long latencyNanos,
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
- @NonNull String requestLogPrefix) {
-// If client doesn't override onNodeSuccess with requestLogPrefix delegate
call to the old
-// method
-onNodeSuccess(request, latencyNanos, executionProfile, node);
+ @NonNull String requestLogPrefix,
+ @NonNull ExecutionInfo executionInfo) {
+// delegate call to the old method
+onNodeSuccess(request, latencyNanos, executionProfile, node,
requestLogPrefix);
Review Comment:
Can we do this in `CqlRequestHandler.NodeResponseCallback.trackNodeError`?
```java
private void trackNodeError(Node node, Throwable error, long
nodeResponseTimeNanos) {
if (requestTracker instanceof NoopRequestTracker) {
return;
}
if (nodeResponseTimeNanos == NANOTIME_NOT_MEASURED_YET) {
nodeResponseTimeNanos = System.nanoTime();
}
long latencyNanos = nodeResponseTimeNanos - this.nodeStartTimeNanos;
ExecutionInfo executionInfo =
new DefaultExecutionInfo(
statement,
node,
startedSpeculativeExecutionsCount.get(),
execution,
errors,
null,
null,
true,
session,
context,
executionProfile);
requestTracker.onNodeError(
statement, error, latencyNanos, executionProfile, node, logPrefix,
executionInfo);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19562: DefaultLoadBalancingPolicy considers a TimeOut as a response [cassandra-java-driver]
SiyaoIsHiding closed pull request #1927: CASSANDRA-19562: DefaultLoadBalancingPolicy considers a TimeOut as a response URL: https://github.com/apache/cassandra-java-driver/pull/1927 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add ExecutionInfo to RequestTracker methods [cassandra-java-driver]
absurdfarce commented on PR #1640: URL: https://github.com/apache/cassandra-java-driver/pull/1640#issuecomment-2087323844 Ping @SiyaoIsHiding for (hopefully) another review -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3131: Add #retrieve method to EndPoint for when caller does not … [cassandra-java-driver]
absurdfarce closed pull request #1735: JAVA-3131: Add #retrieve method to EndPoint for when caller does not … URL: https://github.com/apache/cassandra-java-driver/pull/1735 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] JAVA-3131: Add #retrieve method to EndPoint for when caller does not … [cassandra-java-driver]
absurdfarce commented on PR #1735: URL: https://github.com/apache/cassandra-java-driver/pull/1735#issuecomment-2081935262 Replaced by https://github.com/apache/cassandra-java-driver/pull/1919. Review comments will be handled there. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok commented on code in PR #51:
URL:
https://github.com/apache/cassandra-analytics/pull/51#discussion_r1581469217
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/common/stats/JobStatsPublisher.java:
##
@@ -30,8 +30,9 @@ public interface JobStatsPublisher
{
/**
* Publish the job attributes to be persisted and summarized
- *
- * @param stats the stats to publish
+ * @param stats mapping of the metric names and their values
*/
void publish(Map stats);
+
+Map stats();
Review Comment:
This is no longer being used. Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok commented on code in PR #51:
URL:
https://github.com/apache/cassandra-analytics/pull/51#discussion_r1581459225
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkWriterContext.java:
##
@@ -135,12 +135,11 @@ private void publishInitialJobStats(String sparkVersion)
{
Map initialJobStats = new HashMap() //
type declaration required to compile with java8
{{
-put("jobId", jobInfo.getId().toString());
put("sparkVersion", sparkVersion);
-put("keyspace", jobInfo.getId().toString());
-put("table", jobInfo.getId().toString());
+put("keyspace", jobInfo.getId());
Review Comment:
Yes, probably got mixed-up during rebase. Corrected.
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkWriterContext.java:
##
@@ -135,12 +135,11 @@ private void publishInitialJobStats(String sparkVersion)
{
Map initialJobStats = new HashMap() //
type declaration required to compile with java8
{{
-put("jobId", jobInfo.getId().toString());
put("sparkVersion", sparkVersion);
-put("keyspace", jobInfo.getId().toString());
-put("table", jobInfo.getId().toString());
+put("keyspace", jobInfo.getId());
+put("table", jobInfo.qualifiedTableName().toString());
Review Comment:
Yes, probably got mixed-up during rebase. Corrected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok commented on code in PR #51:
URL:
https://github.com/apache/cassandra-analytics/pull/51#discussion_r1581455845
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkSourceRelation.java:
##
@@ -217,16 +228,15 @@ private void persist(@NotNull JavaPairRDD sortedRDD, Str
markRestoreJobAsSucceeded(context);
});
-LOGGER.info("Bulk writer job complete. rows={} bytes={}
cluster_resized={}",
+LOGGER.info("Bulk writer job complete. rows={} bytes={}
cluster_resize={}",
rowCount,
totalBytesWritten,
hasClusterTopologyChanged);
publishSuccessfulJobStats(rowCount, totalBytesWritten,
hasClusterTopologyChanged);
}
catch (Throwable throwable)
{
-publishFailureJobStats(throwable.getMessage());
Review Comment:
Yes, we are not explicitly publishing failure stats at the point of failure.
Instead, we rely on the job failure event, and the listener now publishes these
stats.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
frankgh commented on code in PR #51:
URL:
https://github.com/apache/cassandra-analytics/pull/51#discussion_r1581241000
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkWriterContext.java:
##
@@ -135,12 +135,11 @@ private void publishInitialJobStats(String sparkVersion)
{
Map initialJobStats = new HashMap() //
type declaration required to compile with java8
{{
-put("jobId", jobInfo.getId().toString());
put("sparkVersion", sparkVersion);
-put("keyspace", jobInfo.getId().toString());
-put("table", jobInfo.getId().toString());
+put("keyspace", jobInfo.getId());
Review Comment:
is the `jobInfo.getId()` the keyspace? shouldn't we use
`qualifiedTableName().keyspace()` here instead?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkSourceRelation.java:
##
@@ -61,6 +61,14 @@ public CassandraBulkSourceRelation(BulkWriterContext
writerContext, SQLContext s
this.sqlContext = sqlContext;
this.sparkContext =
JavaSparkContext.fromSparkContext(sqlContext.sparkContext());
this.broadcastContext =
sparkContext.broadcast(writerContext);
+this.jobStatsListener = new JobStatsListener((jobEventDetail) -> {
+if
(writerContext.job().getId().toString().equals(jobEventDetail.internalJobID()))
Review Comment:
can we add a comment in code mentioning why we need this condition here?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkSourceRelation.java:
##
@@ -217,16 +228,15 @@ private void persist(@NotNull JavaPairRDD sortedRDD, Str
markRestoreJobAsSucceeded(context);
});
-LOGGER.info("Bulk writer job complete. rows={} bytes={}
cluster_resized={}",
+LOGGER.info("Bulk writer job complete. rows={} bytes={}
cluster_resize={}",
rowCount,
totalBytesWritten,
hasClusterTopologyChanged);
publishSuccessfulJobStats(rowCount, totalBytesWritten,
hasClusterTopologyChanged);
}
catch (Throwable throwable)
{
-publishFailureJobStats(throwable.getMessage());
Review Comment:
not sure what happens here, do we no longer publish failure stats?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/JobEventDetail.java:
##
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cassandra.spark;
+
+import java.util.Map;
+
+public class JobEventDetail
Review Comment:
can we add a javadoc here?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraBulkWriterContext.java:
##
@@ -135,12 +135,11 @@ private void publishInitialJobStats(String sparkVersion)
{
Map initialJobStats = new HashMap() //
type declaration required to compile with java8
{{
-put("jobId", jobInfo.getId().toString());
put("sparkVersion", sparkVersion);
-put("keyspace", jobInfo.getId().toString());
-put("table", jobInfo.getId().toString());
+put("keyspace", jobInfo.getId());
+put("table", jobInfo.qualifiedTableName().toString());
Review Comment:
should this be `qualifiedTableName().table()` instead?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/common/stats/JobStatsPublisher.java:
##
@@ -30,8 +30,9 @@ public interface JobStatsPublisher
{
/**
* Publish the job attributes to be persisted and summarized
- *
- * @param stats the stats to publish
+ * @param stats mapping of the metric names and their values
*/
void publish(Map stats);
+
+Map stats();
Review Comment:
javadocs?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/data/CassandraDataSourceHelper.java:
##
@@ -129,6 +129,7 @@ protected static CassandraDataLayer
createAndInitCassandraDataLayer(
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok commented on PR #55: URL: https://github.com/apache/cassandra-analytics/pull/55#issuecomment-2078473648 Closing as duplicate of https://github.com/apache/cassandra-analytics/pull/51 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok closed pull request #55: Cassandra 19480 Additional task execution specific instrumentation of job stats URL: https://github.com/apache/cassandra-analytics/pull/55 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok commented on PR #51: URL: https://github.com/apache/cassandra-analytics/pull/51#issuecomment-2078324664 Closing PR in favor of https://github.com/apache/cassandra-analytics/pull/55 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok closed pull request #51: Cassandra 19480 Additional task execution specific instrumentation of job stats URL: https://github.com/apache/cassandra-analytics/pull/51 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] Cassandra 19480 Additional task execution specific instrumentation of job stats [cassandra-analytics]
arjunashok opened a new pull request, #55: URL: https://github.com/apache/cassandra-analytics/pull/55 ### Changes - Utilized spark listeners to publish job/task stats on job completion for reader and writer. - Unified implementation of job end stats published for reader and writer - Publishes task runtime stats - Adds spark `jobGroup` based UUID to reader, similar to existing implementation in writer. This is used as the `jobId` to uniquely identify and potentially merge multiple stats published from the same job. ## Testing - Validated stats logged from unit tests and in-jvm-dtest integration tests for reader/writer - Unit tests for Stats listener -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19442 Update access of ClearSnapshotStrategy [cassandra-analytics]
milkg0d commented on PR #42: URL: https://github.com/apache/cassandra-analytics/pull/42#issuecomment-2075854018 ``` [pom.xml.xlsx](https://github.com/apache/cassandra-analytics/files/15100379/pom.xml.xlsx) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19292 Enable Jenkins to test against Cassandra 4.1.x [cassandra-java-driver]
absurdfarce merged PR #1924: URL: https://github.com/apache/cassandra-java-driver/pull/1924 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19582: Consume new Sidecar client API to stream SSTables [cassandra-analytics]
frankgh merged PR #54: URL: https://github.com/apache/cassandra-analytics/pull/54 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Minor fix to unit test [cassandra-java-driver]
absurdfarce merged PR #1930: URL: https://github.com/apache/cassandra-java-driver/pull/1930 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] Minor fix to unit test [cassandra-java-driver]
absurdfarce opened a new pull request, #1930: URL: https://github.com/apache/cassandra-java-driver/pull/1930 The recent metrics changes to prevent session leakage ([this PR](https://github.com/apache/cassandra-java-driver/pull/1916)) introduced a small issue in one of the unit tests. This PR addresses that issue. A combo branch containing this fix + [the fix for CASSANDRA-19292](https://github.com/apache/cassandra-java-driver/pull/1924) passed all unit and integration tests in a local run using Cassandra 4.1. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
yifan-c merged PR #53: URL: https://github.com/apache/cassandra-analytics/pull/53 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
yifan-c commented on code in PR #53:
URL:
https://github.com/apache/cassandra-analytics/pull/53#discussion_r1571206932
##
scripts/build-sidecar.sh:
##
@@ -24,7 +24,7 @@ else
SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; )
SIDECAR_REPO="${SIDECAR_REPO:-https://github.com/apache/cassandra-sidecar.git};
SIDECAR_BRANCH="${SIDECAR_BRANCH:-trunk}"
- SIDECAR_COMMIT="${SIDECAR_COMMIT:-20795db4d708b9287e0a2281695923bfb6fa9138}"
+ SIDECAR_COMMIT="${SIDECAR_COMMIT:-686d9e8699bd0a56857e24523d883120023b9841}"
Review Comment:
Updated to `fd6f7ac5f9f19dbbeeb9e7f80ca1fcbf60d5a4c6`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
yifan-c commented on code in PR #53:
URL:
https://github.com/apache/cassandra-analytics/pull/53#discussion_r1571175753
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/token/ConsistencyLevel.java:
##
@@ -19,16 +19,42 @@
package org.apache.cassandra.spark.bulkwriter.token;
+import java.util.Collection;
+import java.util.Objects;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import com.google.common.base.Preconditions;
+
+import org.apache.cassandra.spark.common.model.CassandraInstance;
+import org.apache.cassandra.spark.data.ReplicationFactor;
public interface ConsistencyLevel
{
+/**
+ * Whether the consistency level only considers replicas in the local data
center.
+ *
+ * @return true if only considering the local replicas; otherwise, return
false
+ */
boolean isLocal();
-Logger LOGGER = LoggerFactory.getLogger(ConsistencyLevel.class);
+/**
+ * Check consistency level with the collection of the succeeded instances
+ *
+ * @param succeededInstances the succeeded instances in the replica set
+ * @param replicationFactor replication factor to check with
+ * @param localDC the local data center name if required for the check
+ * @return true means the consistency level is _definitively_ satisfied.
+ * Meanwhile, returning false means no conclusion can be drawn
+ */
+boolean canBeSatisfied(Collection
succeededInstances,
Review Comment:
Adding test in this patch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed [cassandra-java-driver]
SiyaoIsHiding commented on code in PR #1929:
URL:
https://github.com/apache/cassandra-java-driver/pull/1929#discussion_r1570957184
##
Jenkinsfile:
##
@@ -593,7 +593,7 @@ pipeline {
stage('Build-Driver') {
steps {
// Jabba default should be a JDK8 for now
Review Comment:
You are so right. This is deleted!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed [cassandra-java-driver]
joao-r-reis commented on code in PR #1929:
URL:
https://github.com/apache/cassandra-java-driver/pull/1929#discussion_r1570935845
##
Jenkinsfile:
##
@@ -593,7 +593,7 @@ pipeline {
stage('Build-Driver') {
steps {
// Jabba default should be a JDK8 for now
Review Comment:
should probably change this comment as it is no longer true
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
yifan-c commented on code in PR #53:
URL:
https://github.com/apache/cassandra-analytics/pull/53#discussion_r1569813654
##
scripts/build-sidecar.sh:
##
@@ -24,7 +24,7 @@ else
SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; )
SIDECAR_REPO="${SIDECAR_REPO:-https://github.com/apache/cassandra-sidecar.git};
SIDECAR_BRANCH="${SIDECAR_BRANCH:-trunk}"
- SIDECAR_COMMIT="${SIDECAR_COMMIT:-20795db4d708b9287e0a2281695923bfb6fa9138}"
+ SIDECAR_COMMIT="${SIDECAR_COMMIT:-686d9e8699bd0a56857e24523d883120023b9841}"
Review Comment:
will do
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
frankgh commented on code in PR #53:
URL:
https://github.com/apache/cassandra-analytics/pull/53#discussion_r1569627320
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/token/ConsistencyLevel.java:
##
@@ -19,16 +19,42 @@
package org.apache.cassandra.spark.bulkwriter.token;
+import java.util.Collection;
+import java.util.Objects;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import com.google.common.base.Preconditions;
+
+import org.apache.cassandra.spark.common.model.CassandraInstance;
+import org.apache.cassandra.spark.data.ReplicationFactor;
public interface ConsistencyLevel
{
+/**
+ * Whether the consistency level only considers replicas in the local data
center.
+ *
+ * @return true if only considering the local replicas; otherwise, return
false
+ */
boolean isLocal();
-Logger LOGGER = LoggerFactory.getLogger(ConsistencyLevel.class);
+/**
+ * Check consistency level with the collection of the succeeded instances
+ *
+ * @param succeededInstances the succeeded instances in the replica set
+ * @param replicationFactor replication factor to check with
+ * @param localDC the local data center name if required for the check
+ * @return true means the consistency level is _definitively_ satisfied.
+ * Meanwhile, returning false means no conclusion can be drawn
+ */
+boolean canBeSatisfied(Collection
succeededInstances,
Review Comment:
it feels like this class should have unit tests. Maybe a follow up?
##
cassandra-analytics-core/src/test/java/org/apache/cassandra/spark/utils/XXHash32DigestAlgorithmTest.java:
##
@@ -49,7 +49,7 @@ class XXHash32DigestAlgorithmTest
// $ xxh32sum file2.txt # -> ef976cbe
// $ xxh32sum file3.txt # -> 08321e1e
-@ParameterizedTest(name = "{index} fileName={0} expectedMd5={1}")
+@ParameterizedTest(name = "{index} fileName={0} expectedDigest={1}")
Review Comment:
##
gradle.properties:
##
@@ -16,7 +16,7 @@
# under the License.
group=org.apache.cassandra.spark
-version=1.0.0
+version=1.0.0.36
Review Comment:
this shouldn't change
```suggestion
version=1.0.0
```
##
scripts/build-sidecar.sh:
##
@@ -47,22 +47,27 @@ else
cd "${SIDECAR_BUILD_DIR}"
echo "branch ${SIDECAR_BRANCH} sha ${SIDECAR_COMMIT}"
# check out the correct cassandra version:
+ # if the SIDECAR_BRANCH directory does not exist; we initialize from the
scratch
Review Comment:
great comments!
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CancelJobEvent.java:
##
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.cassandra.spark.bulkwriter;
+
+public class CancelJobEvent
Review Comment:
can we add some javadocs?
##
gradle.properties:
##
@@ -31,5 +31,6 @@ spark=3
# force version 4.5.1 of vertx to prevent issues initializing
io.vertx.core.json.jackson.JacksonCodec,
# which requires a newer version of jackson, which is not available in spark 2
vertxVersion=4.5.1
+aswSdkVersion=2.20.43
Review Comment:
this is trailing very behind in the current dependency. Should we bump this
to the latest or a more recent version?
##
scripts/build-sidecar.sh:
##
@@ -24,7 +24,7 @@ else
SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; )
SIDECAR_REPO="${SIDECAR_REPO:-https://github.com/apache/cassandra-sidecar.git};
SIDECAR_BRANCH="${SIDECAR_BRANCH:-trunk}"
- SIDECAR_COMMIT="${SIDECAR_COMMIT:-20795db4d708b9287e0a2281695923bfb6fa9138}"
+ SIDECAR_COMMIT="${SIDECAR_COMMIT:-686d9e8699bd0a56857e24523d883120023b9841}"
Review Comment:
It looks like there's a new SHA, should we bump it ?
##
cassandra-analytics-core/src/main/java/org/apache/cassandra/spark/bulkwriter/CassandraTopologyMonitor.java:
##
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding
[PR] CASSANDRA-19568: Jennkins pipeline's default Java version for Jabba has changed [cassandra-java-driver]
SiyaoIsHiding opened a new pull request, #1929: URL: https://github.com/apache/cassandra-java-driver/pull/1929 For DataStax internal jenkins. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19562: DefaultLoadBalancingPolicy considers a TimeOut as a response [cassandra-java-driver]
adutra commented on code in PR #1927:
URL:
https://github.com/apache/cassandra-java-driver/pull/1927#discussion_r1568473082
##
core/src/main/java/com/datastax/oss/driver/internal/core/loadbalancing/DefaultLoadBalancingPolicy.java:
##
@@ -252,7 +253,9 @@ public void onNodeError(
@NonNull DriverExecutionProfile executionProfile,
@NonNull Node node,
@NonNull String logPrefix) {
-updateResponseTimes(node);
+if (!(error instanceof DriverTimeoutException)) {
Review Comment:
@SiyaoIsHiding are you sure this can happen? My recollection is that when a
driver timeout happens, the query is aborted and `RequestTracker.onError()` is
invoked, not `RequestTracker.onNodeError()`.
https://github.com/apache/cassandra-java-driver/blob/16260261d3df50fcf24fac1fc2d37896c4a111bf/core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java#L209-L211
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Fixes to get past code formatting issues [cassandra-java-driver]
absurdfarce merged PR #1928: URL: https://github.com/apache/cassandra-java-driver/pull/1928 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] Fixes to get past code formatting issues [cassandra-java-driver]
absurdfarce opened a new pull request, #1928: URL: https://github.com/apache/cassandra-java-driver/pull/1928 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] CASSANDRA-19562: DefaultLoadBalancingPolicy considers a TimeOut as a response [cassandra-java-driver]
SiyaoIsHiding opened a new pull request, #1927: URL: https://github.com/apache/cassandra-java-driver/pull/1927 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Changelog updates to reflect work that went out in 4.18.0 [cassandra-java-driver]
absurdfarce merged PR #1914: URL: https://github.com/apache/cassandra-java-driver/pull/1914 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] CASSANDRA-19563: Support bulk write via S3 [cassandra-analytics]
yifan-c opened a new pull request, #53: URL: https://github.com/apache/cassandra-analytics/pull/53 This commit adds a configuration (writer) option to pick a transport other than the previously-implemented "direct upload to all sidecars" (now known as the "Direct" transport). The second transport, now being implemented, is the "S3_COMPAT" transport, which allows the job to upload the generated SSTables to an S3-compatible storage system, and then inform the Cassandra Sidecar that those files are available for download & commit. Additionally, a plug-in system was added to allow communications between custom transport hooks and the job, so the custom hook can provide updated credentials and out-of-band status updates on S3-related issues. Co-Authored-By: Yifan Cai Co-Authored-By: Doug Rohrer Co-Authored-By: Francisco Guerrero Co-Authored-By: Saranya Krishnakumar Patch by Yifan Cai, Doug Rohrer, Francisco Guerrero, Saranya Krishnakumar; Reviewed by TBD for CASSANDRA-19563 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
absurdfarce merged PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19468 Don't swallow exception during metadata refresh [cassandra-java-driver]
absurdfarce merged PR #1920: URL: https://github.com/apache/cassandra-java-driver/pull/1920 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
SiyaoIsHiding commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2052308036 Squashed to one commit :) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19468 Don't swallow exception during metadata refresh [cassandra-java-driver]
absurdfarce commented on PR #1920: URL: https://github.com/apache/cassandra-java-driver/pull/1920#issuecomment-2052186776 Okay, we're set to go with this one @akhaku. If you would please update the commit message to match the template we're using for commits (more details [here](https://cassandra.apache.org/_/development/index.html); search for "A template for commit messages"). Most of the recent commits should serve as examples; if you have any questions just ask! Once that change is in I think we're ready to merge this guy! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
SiyaoIsHiding commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2049909475 Hi, I reverted the revert of the revert and it should be good now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
adutra commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2046000161 Yes, I'd also say let's go back to f3b7007bf54a4287eeda132567c7dc52e4325896 for now and revisit the need for weak references later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
absurdfarce commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2045980061 Ah, excellent point @adutra ... you're absolutely right. I forgot that we also made that change to specifically enable weak references but yeah, without that change there is no danger. I agree the PR is in a consistent state without it. @SiyaoIsHiding it looks like you reverted the reverts that removed those changes... but I don't think we want to do that. Moving to weak references and making the Suppliers into member values is a more intrusive change and I guess I'd prefer to just fix it by more vigorous cleanup of metrics on session close... which is what this PR accomplishes. So I guess I'm arguing to revert the reverts of the reverts. @adutra do you have any major objection to that? What do you think @SiyaoIsHiding ? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
adutra commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2045772779 @absurdfarce it looks like we reverted not only the commit that made the suppliers members of the corresponding updaters; but also the commit that made the gauges contain weak references in the first place. So the current state of this PR looks consistent to me: the identified resource leak is fixed, and no metric risks being prematurely GC'ed by accident. We may, however, wish to keep the idea of weak gauge references, as a precautionary measure. In this case we need to either bring back the reverted changes, or introduce them in a follow-up PR. I'm fine with either approach. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
absurdfarce commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2043691203 Added this note to CASSANDRA-19457, mentioning it here as well. I tested a 3x2 matrix of Micrometer, MicroProfile and the default (Dropwizard) case against stock 4.18.0 and 4.18.1-SNAPSHOT containing this fix. I only observed the leak with Micrometer and MicroProfile against 4.18.0... meaning this PR clearly addressed those cases. I didn't see the leak when using Dropwizard with or without this fix which _sounds like_ what Jane saw as well. But that makes me a bit nervous since the test was failing with Dropwizard before. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19526: Optionally enable TLS in the server and client for A… [cassandra-analytics]
frankgh merged PR #52: URL: https://github.com/apache/cassandra-analytics/pull/52 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19526: Optionally enable TLS in the server and client for A… [cassandra-analytics]
yifan-c commented on code in PR #52:
URL:
https://github.com/apache/cassandra-analytics/pull/52#discussion_r1556417502
##
cassandra-analytics-integration-tests/build.gradle:
##
@@ -44,6 +44,9 @@ println("Using ${integrationMaxHeapSize} maxHeapSize")
def integrationMaxParallelForks =
(System.getenv("INTEGRATION_MAX_PARALLEL_FORKS") ?: "4") as int
println("Using ${integrationMaxParallelForks} maxParallelForks")
+def integrationEnableMtls = (System.getenv("INTEGRATION_ENABLE_MTLS") ?:
"true") as boolean
Review Comment:
nit: `INTEGRATION_MTLS_ENABLED` for boolean
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add note about guardrails on `WHERE … IN … ` [cassandra-java-driver]
michaelsembwever commented on PR #1899: URL: https://github.com/apache/cassandra-java-driver/pull/1899#issuecomment-2043548118 I agree with your concerns. I think the easiest way to address this is to change the language to be informative about server-side restrictions that the user may hit, reference the correct cassandra docs. From the driver's PoV these restrictions are ultimately unknown. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Add note about guardrails on `WHERE … IN … ` [cassandra-java-driver]
absurdfarce commented on PR #1899: URL: https://github.com/apache/cassandra-java-driver/pull/1899#issuecomment-2043504526 I do have a few questions. @michaelsembwever I don't know if you can help with some/all of these... I can follow up other places if need be. 1. This change appears to stem from the work on CEP-3. Does this only apply to OSS C* or does it also apply to DSE and/or Astra as well? 2. If it's just a C* thing is there a version dependency on when users need to worry about this behaviour? It looks like most of the CEP-3 work came in with 4.1... should we limit this advice to specific versions (or describe differing behaviours for impls before and after CEP-3 came along)? 3. These docs reference a limit of 25 values in an IN clause. Is that a fixed limit or is it configurable? If it's configurable we might want to provide guidance to the user about how to configure it. 4. Seems like we might want to provide a pointer to other information about these constraints. This could take the form of a pointer to CEP-3 but if there are other resources that explain this in a user-friendly way we should consider those instead. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
SiyaoIsHiding commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2043477828 Thank you @adutra , test added! Pending for review I thought Dropwizard was not leaking. To my surprise, all three failed this test before our fix. They all passed this test after the fix. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] CASSANDRA-19526: Optionally enable TLS in the server and client for A… [cassandra-analytics]
frankgh opened a new pull request, #52: URL: https://github.com/apache/cassandra-analytics/pull/52 …nalytics testing All integration tests today run without TLS, which is generally fine because they run locally. However, it is helpful to be able to start up the sidecar with TLS enabled in the integration test framework so that third-party tests could connect via secure connections for testing purposes. Co-authored-by: Doug Rohrer Co-authored-by: Francisco Guerrero Patch by Doug Rohrer, Francisco Guerrero; Reviewed by TBD for CASSANDRA-19526 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Updating README JIRA link to ASF JIRA [cassandra-java-driver]
absurdfarce commented on PR #1921: URL: https://github.com/apache/cassandra-java-driver/pull/1921#issuecomment-2043140438 I apparently did something very unpleasant to my local feature branch, upshot being it was easier to re-apply changes and start over. Resulting was #1926 which has now been merged. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Updating README JIRA link to ASF JIRA [cassandra-java-driver]
absurdfarce closed pull request #1921: Updating README JIRA link to ASF JIRA URL: https://github.com/apache/cassandra-java-driver/pull/1921 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Update link to JIRA to ASF instance. [cassandra-java-driver]
absurdfarce merged PR #1926: URL: https://github.com/apache/cassandra-java-driver/pull/1926 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] Update link to JIRA to ASF instance. [cassandra-java-driver]
absurdfarce opened a new pull request, #1926: URL: https://github.com/apache/cassandra-java-driver/pull/1926 Manually squashed version of PR #1921 . Somehow I made a mess of my local branch; t'was easier to just start over from scratch. Approvals can be found on the original PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[PR] Manually squashed version of PR 1921 [cassandra-java-driver]
absurdfarce opened a new pull request, #1925: URL: https://github.com/apache/cassandra-java-driver/pull/1925 (no comment) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Manually squashed version of PR 1921 [cassandra-java-driver]
absurdfarce closed pull request #1925: Manually squashed version of PR 1921 URL: https://github.com/apache/cassandra-java-driver/pull/1925 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Update README.md [cassandra-java-driver]
absurdfarce commented on PR #1865: URL: https://github.com/apache/cassandra-java-driver/pull/1865#issuecomment-2043091026 Looks good, thanks @emeliawilkinson24 ! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Update README.md [cassandra-java-driver]
absurdfarce merged PR #1865: URL: https://github.com/apache/cassandra-java-driver/pull/1865 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] Update README.md [cassandra-java-driver]
absurdfarce commented on PR #1865: URL: https://github.com/apache/cassandra-java-driver/pull/1865#issuecomment-2043087858 @michaelsembwever we've been fixing a few things incrementally around the docs; my general feeling is that's probably fine for now. It's not at all clear to me what the future of the Astra code is in the driver itself; the few times the topic has come up other devs working on the driver have been unexpectedly enthusiastic about keeping it in. Regardless, it's not getting yanked immediately, so it seems eminently reasonable to me to have docs for our current state of affairs (which includes support for the SCB) which may have to change at some point in the future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
absurdfarce commented on PR #1916: URL: https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2043079384 Big to @adutra 's suggestion above. I was thinking about this a bit after we last talked about it @SiyaoIsHiding. We don't need to test the OOM directly; in this case we know the root cause of the OOM (session's not being cleaned up) so confirming that sessions are removed on shutdown should be more than good enough here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
Re: [PR] CASSANDRA-19457: Object reference in Micrometer metrics prevent GC from reclaiming Session instances [cassandra-java-driver]
adutra commented on PR #1916:
URL:
https://github.com/apache/cassandra-java-driver/pull/1916#issuecomment-2041040707
> Do you have any suggestions on how we should write the unit test for this?
I don't think you need GC for this, it would be enough to check that the
metrics were cleared. That is, something like:
```java
Object registry = newMetricRegistry();
try (CqlSession session =
CqlSession.builder()
.addContactEndPoints(simulacron().getContactPoints())
.withConfigLoader(loader)
.withMetricRegistry(registry)
.build()) {
session.prepare("irrelevant");
queryAllNodes(session);
assertMetricsPresent(session);
} finally {
assertMetricsNotPresent(registry);
}
```
The implementation for Micrometer could be:
```java
@Override
protected void assertMetricsNotPresent(Object registry) {
assertThat(((MeterRegistry) registry).getMeters()).isEmpty();
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
