Re: [PR] CASSSIDECAR-209: HealthCheckPeriodicTask leak in Sidecar [cassandra-sidecar]
JeetKunDoug merged PR #191: URL: https://github.com/apache/cassandra-sidecar/pull/191 -- 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: pr-unsubscr...@cassandra.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org For additional commands, e-mail: pr-h...@cassandra.apache.org
Re: [PR] CASSSIDECAR-209: HealthCheckPeriodicTask leak in Sidecar [cassandra-sidecar]
JeetKunDoug commented on code in PR #191:
URL: https://github.com/apache/cassandra-sidecar/pull/191#discussion_r1953349725
##
server/src/main/java/org/apache/cassandra/sidecar/server/Server.java:
##
@@ -302,11 +303,15 @@ protected void validate()
*/
protected Future scheduleInternalPeriodicTasks(String deploymentId)
{
-periodicTaskExecutor.schedule(new HealthCheckPeriodicTask(vertx,
-
sidecarConfiguration,
-
instancesMetadata,
-
executorPools,
- metrics));
+HealthCheckPeriodicTask healthCheckPeriodicTask = new
HealthCheckPeriodicTask(
+sidecarConfiguration,
+
instancesMetadata,
+
executorPools,
+ metrics);
Review Comment:
Done
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
Re: [PR] CASSSIDECAR-209: HealthCheckPeriodicTask leak in Sidecar [cassandra-sidecar]
JeetKunDoug commented on code in PR #191:
URL: https://github.com/apache/cassandra-sidecar/pull/191#discussion_r1953345587
##
scripts/build-shaded-dtest-jar-local.sh:
##
@@ -26,7 +26,7 @@ CASSANDRA_VERSION=$(cat build.xml | grep 'property
name="base.version"' | awk -F
GIT_HASH=$(git rev-parse --short HEAD)
DTEST_ARTIFACT_ID=${ARTIFACT_NAME}-local
DTEST_JAR_DIR="$(dirname "${SCRIPT_DIR}/")/dtest-jars"
-
+DTEST_JAR_DIR=${CASSANDRA_DEP_DIR:-$DTEST_JAR_DIR}
Review Comment:
This builds a jar with `dtest-M.m.p.jar` in the shared directory _if you set
the environment variable_, and the local dir if not - either way, they files
are versioned. If you happen to use the shared dependencies directory, you
often don't have to rebuild C* versions. A previous patch started to _use_ the
shared directory in _gradle_ but didn't include it here, so I made the two
consistent. I happen to like not having several dozen copies of C* builds
floating around my system personally, so I use it and have never had issues...
just don't set the `CASSANDRA_DEP_DIR` environment variable if you don't want
to use it.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
Re: [PR] CASSSIDECAR-209: HealthCheckPeriodicTask leak in Sidecar [cassandra-sidecar]
yifan-c commented on code in PR #191:
URL: https://github.com/apache/cassandra-sidecar/pull/191#discussion_r1953112682
##
scripts/build-shaded-dtest-jar-local.sh:
##
@@ -26,7 +26,7 @@ CASSANDRA_VERSION=$(cat build.xml | grep 'property
name="base.version"' | awk -F
GIT_HASH=$(git rev-parse --short HEAD)
DTEST_ARTIFACT_ID=${ARTIFACT_NAME}-local
DTEST_JAR_DIR="$(dirname "${SCRIPT_DIR}/")/dtest-jars"
-
+DTEST_JAR_DIR=${CASSANDRA_DEP_DIR:-$DTEST_JAR_DIR}
Review Comment:
I do not think the dtest jars should be placed in a central place and
shared. The projects can depend on different SHAs. Unlike the local maven repo,
where the jars can be distinguished by version. Sharing the dtest jars could
cause confusion.
It is unrelated to the patch, just to bring it up, do not need to address.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
Re: [PR] CASSSIDECAR-209: HealthCheckPeriodicTask leak in Sidecar [cassandra-sidecar]
frankgh commented on code in PR #191:
URL: https://github.com/apache/cassandra-sidecar/pull/191#discussion_r1953101868
##
server/src/main/java/org/apache/cassandra/sidecar/server/Server.java:
##
@@ -302,11 +303,15 @@ protected void validate()
*/
protected Future scheduleInternalPeriodicTasks(String deploymentId)
{
-periodicTaskExecutor.schedule(new HealthCheckPeriodicTask(vertx,
-
sidecarConfiguration,
-
instancesMetadata,
-
executorPools,
- metrics));
+HealthCheckPeriodicTask healthCheckPeriodicTask = new
HealthCheckPeriodicTask(
+sidecarConfiguration,
+
instancesMetadata,
+
executorPools,
+ metrics);
Review Comment:
Can we fix the formatting here?
```suggestion
HealthCheckPeriodicTask healthCheckPeriodicTask = new
HealthCheckPeriodicTask(sidecarConfiguration,
instancesMetadata,
executorPools,
metrics);
```
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
