[GitHub] [geode] rhoughton-pivot merged pull request #7797: GEODE-10378: Change PRs to default to JDK11 only
rhoughton-pivot merged PR #7797: URL: https://github.com/apache/geode/pull/7797 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey merged pull request #975: GEODE-10382: Fix Windows build in CI pipeline
pdxcodemonkey merged PR #975: URL: https://github.com/apache/geode-native/pull/975 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] upthewaterspout commented on pull request #7806: GEODE-10388: create better output filter for srcDist task
upthewaterspout commented on PR #7806: URL: https://github.com/apache/geode/pull/7806#issuecomment-1157945426 Oh, wait, I see. You are trying to exclude the whole directory, but only if it is a directory and not a file. Ok. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] rhoughton-pivot opened a new pull request, #7806: GEODE-10388: create better output filter for srcDist task
rhoughton-pivot opened a new pull request, #7806: URL: https://github.com/apache/geode/pull/7806 Replace the brittle exclude list for `build` and `out` directories with a closure containing comparison logic for file-type and name. Makes sure that regular files named `out` are still archived. ### For all changes: - [x] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [x] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [x] Is your initial contribution a single, squashed commit? - [x] Does `gradlew build` run cleanly? - [n/a] Have you written or updated unit tests to verify your changes? - [n/a] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz closed pull request #7784: [DRAFT DO NOT REVIEW] Wip/handshake locks
albertogpz closed pull request #7784: [DRAFT DO NOT REVIEW] Wip/handshake locks URL: https://github.com/apache/geode/pull/7784 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] jchen21 merged pull request #7804: GEODE-10384: Add stack trace to logging
jchen21 merged PR #7804: URL: https://github.com/apache/geode/pull/7804 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] jchen21 opened a new pull request, #7804: Add stack trace to logging
jchen21 opened a new pull request, #7804: URL: https://github.com/apache/geode/pull/7804 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7802: (no JIRA ticket) user guide Security section: Typo & format fixes
davebarnes97 merged PR #7802: URL: https://github.com/apache/geode/pull/7802 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett opened a new pull request, #7803: GEODE-10354: Convert enum like classes to enum.
pivotal-jbarrett opened a new pull request, #7803: URL: https://github.com/apache/geode/pull/7803 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode-native] pdxcodemonkey opened a new pull request, #975: GEODE-10382: Fix Windows build in CI pipeline
pdxcodemonkey opened a new pull request, #975: URL: https://github.com/apache/geode-native/pull/975 Add echo command to cmake configuration step, for debugging -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] jinmeiliao merged pull request #7801: GEODE-10380: use waitingThreadPool to notify dispatcher at re_auth
jinmeiliao merged PR #7801: URL: https://github.com/apache/geode/pull/7801 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] jinmeiliao commented on pull request #7801: GEODE-10380: use waitingThreadPool to notify dispatcher at re_auth
jinmeiliao commented on PR #7801: URL: https://github.com/apache/geode/pull/7801#issuecomment-1156700605 Thanks! I will merge this after more thorough tests are 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mivanac commented on pull request #7749: GEODE-10020: For Ping task avoid registering new destination endpoint
mivanac commented on PR #7749: URL: https://github.com/apache/geode/pull/7749#issuecomment-1156604322 Hi just a reminder, could you review this PR. This was reverted PR with modifications for failing internal test. Thanks -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mivanac commented on pull request #7664: Newfeature2/geode 9484
mivanac commented on PR #7664: URL: https://github.com/apache/geode/pull/7664#issuecomment-1156600848 Hi @Bill, @echobravopapa, @agingade, @kamilla1201 and @pivotal-jbarrett just a reminder, could you review this PR. This was reverted PR with modifications for failing internal test. Thanks -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mivanac commented on a diff in pull request #7323: GEODE-9997: added ParallelQueueSetPossibleDuplicateMessage
mivanac commented on code in PR #7323:
URL: https://github.com/apache/geode/pull/7323#discussion_r898081743
##
geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java:
##
@@ -215,6 +216,34 @@ enum OrderPolicy {
*/
void startWithCleanQueue();
+
+ /**
+ * Prepare GatewaySender for closing of Cache.
+ *
+ *
+ * Implementation of new API in ParallelGatewaySenderImpl:
+ *
+ *
+ *
+ * public void prepareForStop() {
+ * if (!isRunning()) {
+ * return;
+ * }
+ * pause();
+ * if (eventProcessor != null !eventProcessor.isStopped()) {
+ * eventProcessor.prepareForStopProcessing();
+ * }
+ * }
+ *
+ *
+ *
+ *
+ * Invoked at closing of cache.
+ *
+ */
+ @Experimental
+ void prepareForStop();
Review Comment:
Hi @kirklund , are changes OK with 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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jvarenina commented on pull request #7725: GEODE-10338: Fix LogWriterAppender shutdown
jvarenina commented on PR #7725:
URL: https://github.com/apache/geode/pull/7725#issuecomment-1156398017
When created in the same VM, all cache instances use the same
LogWriterAppender service. Therefore all tests that create multiple caches and
try to close them will fail with `NullpointerExecption `(the `stopSession()`
method will be executed more than once for the same `LogWriterAppender`).
For example check MultipleCacheJUnitTest.cacheCreateTwoPeerCaches test case:
```
Jale create cache1
Jale LOGWRITER with instance
org.apache.geode.logging.log4j.internal.impl.LogWriterAppender@49925e47:LOGWRITER
{eagerMemberName=null, lazyMemberName=appendLog=true, security=false,
paused=false,
loggingSessionRegistry=org.apache.geode.logging.internal.LoggingSessionRegistryProvider@4ae90315,
logWriter=org.apache.geode.logging.log4j.internal.impl.NullLogWriter@6b0572fa,
debug=false} has logWriter object:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@6b0572fa
Jale SECURITYLOGWRITER with instance
org.apache.geode.logging.log4j.internal.impl.LogWriterAppender@7ed478ab:SECURITYLOGWRITER
{eagerMemberName=null, lazyMemberName=appendLog=true, security=true,
paused=false,
loggingSessionRegistry=org.apache.geode.logging.internal.LoggingSessionRegistryProvider@4ae90315,
logWriter=org.apache.geode.logging.log4j.internal.impl.NullLogWriter@3a981478,
debug=false} has logWriter object:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@3a981478
Jale create cache2
Jale LOGWRITER with instance
org.apache.geode.logging.log4j.internal.impl.LogWriterAppender@49925e47:LOGWRITER
{eagerMemberName=null, lazyMemberName=appendLog=true, security=false,
paused=false,
loggingSessionRegistry=org.apache.geode.logging.internal.LoggingSessionRegistryProvider@4ae90315,
logWriter=org.apache.geode.logging.log4j.internal.impl.NullLogWriter@78cc6db3,
debug=false} has logWriter object:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@78cc6db3
Jale SECURITYLOGWRITER with instance
org.apache.geode.logging.log4j.internal.impl.LogWriterAppender@7ed478ab:SECURITYLOGWRITER
{eagerMemberName=null, lazyMemberName=appendLog=true, security=true,
paused=false,
loggingSessionRegistry=org.apache.geode.logging.internal.LoggingSessionRegistryProvider@4ae90315,
logWriter=org.apache.geode.logging.log4j.internal.impl.NullLogWriter@3839a6d9,
debug=false} has logWriter object:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@3839a6d9
Jale closing: cache1
Jale stopSession:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@78cc6db3
Jale stopSession:
org.apache.geode.logging.log4j.internal.impl.NullLogWriter@3839a6d9
Jale closing: cache2
Jale stopSession: null
java.lang.NullPointerException
```
I think it would be best to return the check whether `logWriter` is null at
the beginning of `stopSession()` method for these test cases to work.
@kirklund, what do you think?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett merged pull request #7688: GEODE-10326: Convert MessageType to enum.
pivotal-jbarrett merged PR #7688: URL: https://github.com/apache/geode/pull/7688 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] DonalEvans merged pull request #7631: GEODE-10261: VMProvider.invokeAsync uses appropriate parameterization.
DonalEvans merged PR #7631: URL: https://github.com/apache/geode/pull/7631 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 opened a new pull request, #7802: (no JIRA ticket) user guide Security section: Typo & format fixes
davebarnes97 opened a new pull request, #7802: URL: https://github.com/apache/geode/pull/7802 Doc format fixes. Back port to 1.15 ASAP. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7368: GEODE-10055: fix AbstractLauncher to print info and debug with stdout
pivotal-jbarrett commented on PR #7368: URL: https://github.com/apache/geode/pull/7368#issuecomment-1155476232 It is common in console programming to output "verbose" output to stderr and "normal" output to stdout. Verbose output being log like stuff and normal output being results of the execution. The rational for this is to be able to split the output like this. ```shell $ hello-world 2>/tmp/hello-world.log Hello World $ cat /tmp/hello-world.log 06-14-2022 10:11:12 PDT - INFO - hello-world starting. 06-14-2022 10:11:12 PDT - WARN - something doesn't feel right today. 06-14-2022 10:11:12 PDT - INFO - hello-world stopping. ``` -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz commented on a diff in pull request #7747: GEODE-10334: Send alert when thread stuck for long
albertogpz commented on code in PR #7747:
URL: https://github.com/apache/geode/pull/7747#discussion_r896655954
##
geode-core/src/main/java/org/apache/geode/internal/monitoring/executor/AbstractExecutor.java:
##
@@ -51,9 +56,25 @@ protected AbstractExecutor(String groupName, long threadID) {
public void handleExpiry(long stuckTime, Map
threadInfoMap) {
incNumIterationsStuck();
+sendAlertForThreadStuckForLong(stuckTime, threadInfoMap);
logger.warn(createThreadReport(stuckTime, threadInfoMap));
}
+ private void sendAlertForThreadStuckForLong(long stuckTime, Map threadInfoMap) {
+if (maxThreadStuckTime <= 0) {
+ return;
+}
+if (threadInfoMap.get(threadID) == null) {
+ return;
+}
+if (stuckForGood) {
+ return;
+}
+if (stuckTime > maxThreadStuckTime) {
+ stuckForGood = true;
+ logger.fatal(createThreadReport(stuckTime, threadInfoMap));
Review Comment:
@kirklund The problem I see with using `error` in this case is that, as you
need to change the `alertLevel` in the `DistributedMXBean` to a lower level,
the number of notifications received will be much higher, something that you do
not necessarily want, for example because more resources will be used.
There are other alerts in Geode issued when a fatal error is sent that do
not necessarily mean a catastrophic failure, for example if the
`ack-severe-threshold` is surpassed and that feature is enabled.
Given that this is also a configurable feature, whoever makes use of it
should know what to expect and how to act (according to the documentation) when
the alert is received so I still think it makes sense to use `severe` to make
sure the alert is always received if the feature is enabled.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] mkevo commented on pull request #7368: GEODE-10055: fix AbstractLauncher to print info and debug with stdout
mkevo commented on PR #7368: URL: https://github.com/apache/geode/pull/7368#issuecomment-1154808661 > I am quite certain this is intentional. Perhaps it is worth a conversation on dev@geode to see if we want to change this. Hi @pivotal-jbarrett, why do you think that this is intentional? I saw that it prints messages like _'Getting Locator status using working directory'_. Not sure that this should be printed as an error, maybe I'm wrong, but need some clarification of your thoughts. Thanks! -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal merged pull request #7770: GEODE-10355: Bump spring-security from 5.2.12 to 5.5.8
onichols-pivotal merged PR #7770: URL: https://github.com/apache/geode/pull/7770 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal merged pull request #7769: GEODE-10355: Bump spring-security from 5.4.8 to 5.6.5
onichols-pivotal merged PR #7769: URL: https://github.com/apache/geode/pull/7769 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller opened a new pull request, #7800: [GEODE-10369] if scratch ssd is used, fall back to n1 machine type.
smgoller opened a new pull request, #7800: URL: https://github.com/apache/geode/pull/7800 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7211: GEODE-10354: Convert enum like classes to enum.
pivotal-jbarrett commented on PR #7211: URL: https://github.com/apache/geode/pull/7211#issuecomment-1154370976 @upthewaterspout, this brings up an excellent question. Does the use of the `Serializable` interface outside of the direct API, which does not utilize it, constitute part of the binary compatibility contract for a public class. Our API or internals do not transmit the classes in question as serializables but the test framework does. As a result of having the `Serializable` interface on the class are we stuck making sure that external use of this class maintains serialization compatibility? If the answer to this is yes, then it will likely put a damper on future mutation of types that either directly or indirectly, like `enum`, implement `Serializable`. There are many issues with versioning `Serializable` classes that may force us to consider `@Deprecated` annotations on these sorts of types and making new types to replace them anytime we want to mutate them. If the answer is no, then how should we communicate this to users? We could define a `@NotSerializable` annotation and Javadoc tag to hopefully inform the user not to use it in serialization. We could implement the write/readObject methods and throw exceptions to prevent it at runtime. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller merged pull request #7798: GEODE-10369 part 2 Fix hardcoded gcp project reference and adjust windows unit test cpu/ram usage.
smgoller merged PR #7798: URL: https://github.com/apache/geode/pull/7798 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller opened a new pull request, #7798: GEODE-10369 part 2 Fix hardcoded gcp project reference and adjust windows unit test cpu/ram usage.
smgoller opened a new pull request, #7798: URL: https://github.com/apache/geode/pull/7798 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal commented on a diff in pull request #7759: Fixes for cleaning GCP resources from pipelines
onichols-pivotal commented on code in PR #7759: URL: https://github.com/apache/geode/pull/7759#discussion_r895956599 ## ci/pipelines/clean_fork_pipelines.sh: ## @@ -22,10 +25,11 @@ while [ -h "$SOURCE" ]; do # resolve $SOURCE until the file is no longer a symli done SCRIPTDIR="$( cd -P "$( dirname "$SOURCE" )" && pwd )" -TARGET=geode +TARGET=concourse.apachegeode-ci.info Review Comment: should be `concourse.apachegeode-ci.info-main` -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz commented on a diff in pull request #7747: GEODE-10334: Send alert when thread stuck for long
albertogpz commented on code in PR #7747:
URL: https://github.com/apache/geode/pull/7747#discussion_r89587
##
geode-core/src/main/java/org/apache/geode/internal/monitoring/executor/AbstractExecutor.java:
##
@@ -51,9 +56,25 @@ protected AbstractExecutor(String groupName, long threadID) {
public void handleExpiry(long stuckTime, Map
threadInfoMap) {
incNumIterationsStuck();
+sendAlertForThreadStuckForLong(stuckTime, threadInfoMap);
logger.warn(createThreadReport(stuckTime, threadInfoMap));
}
+ private void sendAlertForThreadStuckForLong(long stuckTime, Map threadInfoMap) {
+if (maxThreadStuckTime <= 0) {
+ return;
+}
+if (threadInfoMap.get(threadID) == null) {
+ return;
+}
+if (stuckForGood) {
+ return;
+}
+if (stuckTime > maxThreadStuckTime) {
+ stuckForGood = true;
+ logger.fatal(createThreadReport(stuckTime, threadInfoMap));
Review Comment:
@kirklund I think the alert will not be generated by default if `error` is
used. In order to see the alert when using `error`, the `alertLevel` in the
`DistributedMXBean` should be changed from `severe` to a lower level.
See
https://geode.apache.org/docs/guide/114/managing/management/notification_federation_and_alerts.html
Anyhow, I agree with you that it makes sense to use `error` given that
having a thread stuck is not necessarily a catastrophic failure that may result
in data corruption.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] mkevo merged pull request #7643: GEODE-10267: fix creating gw sender with non-existent disk store
mkevo merged PR #7643: URL: https://github.com/apache/geode/pull/7643 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller merged pull request #7796: GEODE-10369: Calculate heavy lifter costs.
smgoller merged PR #7796: URL: https://github.com/apache/geode/pull/7796 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] rhoughton-pivot opened a new pull request, #7797: GEODE-10378: Change PRs to default to JDK11 only
rhoughton-pivot opened a new pull request, #7797: URL: https://github.com/apache/geode/pull/7797 Other JDK will be by request (label) only. Authored-by: Robert Houghton -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller commented on pull request #7796: Geode 10369 - Calculate heavy lifter costs.
smgoller commented on PR #7796: URL: https://github.com/apache/geode/pull/7796#issuecomment-1152775565 > YOU FORGOT TO SQUASH NOM -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] rhoughton-pivot commented on pull request #7796: Geode 10369 - Calculate heavy lifter costs.
rhoughton-pivot commented on PR #7796: URL: https://github.com/apache/geode/pull/7796#issuecomment-1152774931 YOU FORGOT TO SQUASH -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller closed pull request #7796: Geode 10369 - Calculate heavy lifter costs.
smgoller closed pull request #7796: Geode 10369 - Calculate heavy lifter costs. URL: https://github.com/apache/geode/pull/7796 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] smgoller opened a new pull request, #7796: Geode 10369 - Calculate heavy lifter costs.
smgoller opened a new pull request, #7796: URL: https://github.com/apache/geode/pull/7796 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] agingade commented on a diff in pull request #7211: GEODE-10354: Convert enum like classes to enum.
agingade commented on code in PR #7211:
URL: https://github.com/apache/geode/pull/7211#discussion_r894884242
##
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put70Test.java:
##
@@ -328,14 +325,14 @@ void
shouldSetPossibleDuplicateReturnsTrueIfNotRecoveredVersionTagAndWithPersist
@Test
void isRegionWithPersistenceReturnsTrueIfDataPolicyWithPersistence() {
-when(dataPolicy.withPersistence()).thenReturn(true);
+
when(attributes.getDataPolicy()).thenReturn(DataPolicy.PERSISTENT_REPLICATE);
assertThat(put70.isRegionWithPersistence(localRegion)).isTrue();
}
@Test
void
isRegionWithPersistenceReturnsTrueIfIsAccessorAndHavingPersistentMembers() {
-when(dataPolicy.withPersistence()).thenReturn(false);
+when(attributes.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
Review Comment:
Based on the test name; is it expecting it to treat as persistent when it is
accessor but not configured with persistence and has persistent members?
If thats the case; the change seems to be defeating the purpose by not
having:
when(dataPolicy.withPersistence()).thenReturn(false);
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] kirklund commented on a diff in pull request #7747: GEODE-10334: Send alert when thread stuck for long
kirklund commented on code in PR #7747:
URL: https://github.com/apache/geode/pull/7747#discussion_r894864266
##
geode-core/src/main/java/org/apache/geode/internal/monitoring/executor/AbstractExecutor.java:
##
@@ -51,9 +56,25 @@ protected AbstractExecutor(String groupName, long threadID) {
public void handleExpiry(long stuckTime, Map
threadInfoMap) {
incNumIterationsStuck();
+sendAlertForThreadStuckForLong(stuckTime, threadInfoMap);
logger.warn(createThreadReport(stuckTime, threadInfoMap));
}
+ private void sendAlertForThreadStuckForLong(long stuckTime, Map threadInfoMap) {
+if (maxThreadStuckTime <= 0) {
+ return;
+}
+if (threadInfoMap.get(threadID) == null) {
+ return;
+}
+if (stuckForGood) {
+ return;
+}
+if (stuckTime > maxThreadStuckTime) {
+ stuckForGood = true;
+ logger.fatal(createThreadReport(stuckTime, threadInfoMap));
Review Comment:
`fatal` should be reserved for logging about catastrophic failure that may
result in data corruption. The server should probably be taken off-line or
bounced.
`error` should be used to indicate that something failed but there should be
no data corruption and it should generally be ok to allow the server to
continue running.
I think you probably want this log statement to be at `error` level which
will still generate an `alert`.
I'd like to add @dschneider-pivotal as a reviewer to confirm the above.
Darrel, if you want to give me good definitions, I'd be happy to add it to the
Apache Geode Wiki.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jinmeiliao opened a new pull request, #7795: GEODE-10375: improve test to catch doc link update
jinmeiliao opened a new pull request, #7795: URL: https://github.com/apache/geode/pull/7795 improve test in case the doc link gets updated -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal merged pull request #7794: GEODE-10375: update supported api docs link
onichols-pivotal merged PR #7794: URL: https://github.com/apache/geode/pull/7794 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag merged pull request #7779: GEODE-6504: Instant class used.
nabarunnag merged PR #7779: URL: https://github.com/apache/geode/pull/7779 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7211: GEODE-10354: Convert enum like classes to enum.
pivotal-jbarrett commented on code in PR #7211:
URL: https://github.com/apache/geode/pull/7211#discussion_r894777348
##
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put70Test.java:
##
@@ -328,14 +325,14 @@ void
shouldSetPossibleDuplicateReturnsTrueIfNotRecoveredVersionTagAndWithPersist
@Test
void isRegionWithPersistenceReturnsTrueIfDataPolicyWithPersistence() {
-when(dataPolicy.withPersistence()).thenReturn(true);
+
when(attributes.getDataPolicy()).thenReturn(DataPolicy.PERSISTENT_REPLICATE);
assertThat(put70.isRegionWithPersistence(localRegion)).isTrue();
}
@Test
void
isRegionWithPersistenceReturnsTrueIfIsAccessorAndHavingPersistentMembers() {
-when(dataPolicy.withPersistence()).thenReturn(false);
+when(attributes.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
Review Comment:
You fill find lots of places in the old tests where the combinations of
mocks and return values used for `DataPolicy` actually created mutually
exclusive combinations but it didn't matter for the sake of the test. For the
changes here I tried to match up what the test was actually expecting for the
correct combinations that aligned to an enum value.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] upthewaterspout commented on pull request #7211: GEODE-10354: Convert enum like classes to enum.
upthewaterspout commented on PR #7211: URL: https://github.com/apache/geode/pull/7211#issuecomment-1152604125 @pivotal-jbarrett - Here's a test you can play with, based on this PR - https://github.com/upthewaterspout/geode/pull/new/feature/test-compatibility-of-enum-DataPolicy -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7211: GEODE-10354: Convert enum like classes to enum.
pivotal-jbarrett commented on PR #7211: URL: https://github.com/apache/geode/pull/7211#issuecomment-1152603251 > I did a quick test, and it looks like this is a breaking change to the serialization format of this class. I think that's why we haven't made this conversion up to this point: > > ``` > Caused by: java.io.InvalidClassException: cannot bind non-enum descriptor to an enum class >at java.io.ObjectStreamClass.initNonProxy(ObjectStreamClass.java:687) >at java.io.ObjectInputStream.readNonProxyDesc(ObjectInputStream.java:1885) >at java.io.ObjectInputStream.readClassDesc(ObjectInputStream.java:1751) >at java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2042) >at java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1573) >at java.io.ObjectInputStream.readObject(ObjectInputStream.java:431) >at org.apache.geode.internal.InternalDataSerializer.readSerializable(InternalDataSerializer.java:2720) > ``` Which test? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7688: GEODE-10326: Convert MessageType to enum.
pivotal-jbarrett commented on code in PR #7688:
URL: https://github.com/apache/geode/pull/7688#discussion_r894771540
##
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUpdateMessageImpl.java:
##
@@ -1011,7 +1015,7 @@ public void sendTo(DataOutput out) throws IOException {
InternalDataSerializer.writeArrayLength(size, out);
if (size > 0) {
DataSerializer.writeObject(name[0], out);
-DataSerializer.writeObject(op, out);
+DataSerializer.writeObject(op.id, out);
Review Comment:
Ok, I refactored the commits, added some tests around this untested class in
question and fixed the bug pointed out 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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jinmeiliao opened a new pull request, #7794: GEODE-10375: update supported api docs link
jinmeiliao opened a new pull request, #7794: URL: https://github.com/apache/geode/pull/7794 update to springdoc link after [GEODE-10312](https://issues.apache.org/jira/browse/GEODE-10312) -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] dschneider-pivotal opened a new pull request, #7793: removed add-opens for jdk.internal.ref to see what breaks
dschneider-pivotal opened a new pull request, #7793: URL: https://github.com/apache/geode/pull/7793 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] gesterzhou commented on a diff in pull request #7211: GEODE-10354: Convert enum like classes to enum.
gesterzhou commented on code in PR #7211:
URL: https://github.com/apache/geode/pull/7211#discussion_r894707167
##
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put70Test.java:
##
@@ -328,14 +325,14 @@ void
shouldSetPossibleDuplicateReturnsTrueIfNotRecoveredVersionTagAndWithPersist
@Test
void isRegionWithPersistenceReturnsTrueIfDataPolicyWithPersistence() {
-when(dataPolicy.withPersistence()).thenReturn(true);
+
when(attributes.getDataPolicy()).thenReturn(DataPolicy.PERSISTENT_REPLICATE);
assertThat(put70.isRegionWithPersistence(localRegion)).isTrue();
}
@Test
void
isRegionWithPersistenceReturnsTrueIfIsAccessorAndHavingPersistentMembers() {
-when(dataPolicy.withPersistence()).thenReturn(false);
+when(attributes.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
Review Comment:
logically it's the same 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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] DonalEvans merged pull request #7721: GEODE-10329: Handle RejectedExecutionException
DonalEvans merged PR #7721: URL: https://github.com/apache/geode/pull/7721 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7721: GEODE-10329: Handle RejectedExecutionException
pivotal-jbarrett commented on code in PR #7721: URL: https://github.com/apache/geode/pull/7721#discussion_r894035661 ## geode-membership/src/test/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitorTest.java: ## @@ -0,0 +1,63 @@ +/* + * 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.geode.distributed.internal.membership.gms.fd; + +import static org.assertj.core.api.Assertions.assertThatNoException; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.concurrent.RejectedExecutionException; + +import org.junit.Test; Review Comment: Use JUnit 5. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7721: GEODE-10329: Handle RejectedExecutionException
pivotal-jbarrett commented on code in PR #7721:
URL: https://github.com/apache/geode/pull/7721#discussion_r894035523
##
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/fd/GMSHealthMonitor.java:
##
@@ -1425,6 +1425,16 @@ private void sendSuspectRequest(final
List> requests) {
processMessage(smm);
}
+ void doAndIgnoreRejectedExecutionExceptionIfStopping(final Checker checker) {
Review Comment:
Looks like `Runnable` would suffice rather than building our own interface.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] DonalEvans commented on pull request #7721: GEODE-10329: Handle RejectedExecutionException
DonalEvans commented on PR #7721: URL: https://github.com/apache/geode/pull/7721#issuecomment-1151676410 @pivotal-jbarrett Thanks for the advice! I think I managed to implement what you suggested; please let me know if it needs further tweaks. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7688: GEODE-10326: Convert MessageType to enum.
pivotal-jbarrett commented on code in PR #7688:
URL: https://github.com/apache/geode/pull/7688#discussion_r893938054
##
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUpdateMessageImpl.java:
##
@@ -1011,7 +1015,7 @@ public void sendTo(DataOutput out) throws IOException {
InternalDataSerializer.writeArrayLength(size, out);
if (size > 0) {
DataSerializer.writeObject(name[0], out);
-DataSerializer.writeObject(op, out);
+DataSerializer.writeObject(op.id, out);
Review Comment:
Actually on further inspection it is all sort of wonky and probably does
lead to a NPE. There seems to be a break in the original logic around what
makes it `isEmpty()`. If constructed with a `null` name the `isEmpty()` did not
return `true` as it would be expected to. I am going to whip up some unit tests
around this class and correct. Clearly there aren't any tests that hit this
scenario either or at least none that fail. In the original code you would have
had `null` and `0` serialized on the wire for an empty entry like this. `0`
meaning it was a `REQUEST` message. Invoking the `add()` method would have
resulted in the NPE.
Great find!
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7688: GEODE-10326: Convert MessageType to enum.
pivotal-jbarrett commented on code in PR #7688:
URL: https://github.com/apache/geode/pull/7688#discussion_r893928279
##
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUpdateMessageImpl.java:
##
@@ -1011,7 +1015,7 @@ public void sendTo(DataOutput out) throws IOException {
InternalDataSerializer.writeArrayLength(size, out);
if (size > 0) {
DataSerializer.writeObject(name[0], out);
-DataSerializer.writeObject(op, out);
+DataSerializer.writeObject(op.id, out);
Review Comment:
It looks like no, because when constructed with `null` the `name` will also
be `null`. This means that if a message is added then the op is taking from the
first message added, otherwise if it remains empty when serialized the `op.id`
is never referenced. To be absolutely clear though I am going to `@NotNull` in
that constructor, add a new default constructor for the empty case so it isn't
called with `null`, guaranteeing that both `name` and `op` will be `null`
together. Good find!!
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] kirklund opened a new pull request, #7791: GEODE-10366: Cleanup GfshRule created Folder
kirklund opened a new pull request, #7791: URL: https://github.com/apache/geode/pull/7791 PROBLEM Tests using GfshRule default constructor do not delete the contents of the internal Folder created by GfshRule when all tests passed. This results in unnecessary size bloating of the archive file. SOLUTION Ensure that GfshRule handles Folder policy and invokes delete when appropriate. -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] agingade commented on a diff in pull request #7211: GEODE-10354: Convert enum like classes to enum.
agingade commented on code in PR #7211:
URL: https://github.com/apache/geode/pull/7211#discussion_r893861404
##
geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/Put70Test.java:
##
@@ -328,14 +325,14 @@ void
shouldSetPossibleDuplicateReturnsTrueIfNotRecoveredVersionTagAndWithPersist
@Test
void isRegionWithPersistenceReturnsTrueIfDataPolicyWithPersistence() {
-when(dataPolicy.withPersistence()).thenReturn(true);
+
when(attributes.getDataPolicy()).thenReturn(DataPolicy.PERSISTENT_REPLICATE);
assertThat(put70.isRegionWithPersistence(localRegion)).isTrue();
}
@Test
void
isRegionWithPersistenceReturnsTrueIfIsAccessorAndHavingPersistentMembers() {
-when(dataPolicy.withPersistence()).thenReturn(false);
+when(attributes.getDataPolicy()).thenReturn(DataPolicy.PARTITION);
Review Comment:
The older code seems to be looking to know if its configured with persistent
data-policy.
The new change is different than that. Is it what is expected in the test.
In the previous above change; for the same old code its returning
DataPolicy.PERSISTENT_REPLICATE.
Same comment with following two changes in this file.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] mivanac commented on a diff in pull request #7323: GEODE-9997: added ParallelQueueSetPossibleDuplicateMessage
mivanac commented on code in PR #7323:
URL: https://github.com/apache/geode/pull/7323#discussion_r893840797
##
geode-core/src/main/java/org/apache/geode/cache/wan/GatewaySender.java:
##
@@ -215,6 +216,34 @@ enum OrderPolicy {
*/
void startWithCleanQueue();
+
+ /**
+ * Prepare GatewaySender for closing of Cache.
+ *
+ *
+ * Implementation of new API in ParallelGatewaySenderImpl:
+ *
+ *
+ *
+ * public void prepareForStop() {
+ * if (!isRunning()) {
+ * return;
+ * }
+ * pause();
+ * if (eventProcessor != null !eventProcessor.isStopped()) {
+ * eventProcessor.prepareForStopProcessing();
+ * }
+ * }
+ *
+ *
+ *
+ *
+ * Invoked at closing of cache.
+ *
+ */
+ @Experimental
+ void prepareForStop();
Review Comment:
Since changes are only for internal use, impacts moved to
InternalGatewaySender interface.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on pull request #7721: GEODE-10329: Handle RejectedExecutionException
pivotal-jbarrett commented on PR #7721:
URL: https://github.com/apache/geode/pull/7721#issuecomment-1151458738
@DonalEvans what if we refactored a little to a method that takes function
to execute and optionally ignores this specific exception, then we could at
least deterministically test that part of the behavior and change in this PR. 路
```java
private void doAndIgnoreRejectedExecutionExceptionIfStopping(final
SomeFunctionalnterface someFunctionalThing) {
try {
someFunctionalThing.doIt();
} catch (RejectedExecutionException e) {
if (!isStopping) {
throw e;
}
}
}
...
doAndIgnoreRejectedExecutionExceptionIfStopping(() ->
checkExecutor.execute(() -> {
try {
inlineCheckIfAvailable(initiator, cv, true, mbr, reason);
} catch (MembershipClosedException e) {
// shutting down
} catch (Exception e) {
logger.info("Unexpected exception while verifying member", e);
}
}));
...
```
```java
@Test
void doAndIgnoreRejectedExecutionExceptionIfStoppingThrowsWhenNotStopping() {
gmsHealthMonitor.isStopping = true;
assertThatThrowBy(() ->
gmsHealthMonitor.doAndIgnoreRejectedExecutionExceptionIfStopping(() -> {throw
new
RejectedExecutionException();})).isInstanceOf(RejectedExecutionException.class);
}
```
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jinmeiliao merged pull request #7777: GEODE-6489: fix a flaky test
jinmeiliao merged PR #: URL: https://github.com/apache/geode/pull/ -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7688: GEODE-10326: Convert MessageType to enum.
nabarunnag commented on code in PR #7688:
URL: https://github.com/apache/geode/pull/7688#discussion_r893815867
##
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUpdateMessageImpl.java:
##
@@ -1011,7 +1015,7 @@ public void sendTo(DataOutput out) throws IOException {
InternalDataSerializer.writeArrayLength(size, out);
if (size > 0) {
DataSerializer.writeObject(name[0], out);
-DataSerializer.writeObject(op, out);
+DataSerializer.writeObject(op.id, out);
Review Comment:
Is there a possibility of an NPE here or other places if we call members of
op, if we created CqNameToOpSingleEntry with op value as null. As we can see is
possible in the new code change in HAEventWrapper.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7779: GEODE-6504: Instant class used.
pivotal-jbarrett commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893817487
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -84,15 +85,16 @@ public void increaseRegionTtl() throws Exception {
.setRegionTimeToLive(new ExpirationAttributes(secondTtlSeconds,
DESTROY));
await().until(region::isDestroyed);
-assertThat(NANOSECONDS.toSeconds(nanoTime() - startNanos))
-.isGreaterThanOrEqualTo(secondTtlSeconds);
+Instant endInstant = Instant.now();
+assertThat(Duration.between(startInstant, endInstant))
+.isGreaterThanOrEqualTo(Duration.ofSeconds(secondTtlSeconds));
}
@Test
- public void decreaseRegionTtl() throws Exception {
+ public void decreaseRegionTtl() {
int firstTtlSeconds = 5;
Review Comment:
For readability I wonder if this is clearer now that we have introduced
these new time classes:
```java
final Duration firstTtl = Duration.ofSeconds(5);
final Duration secondTtl = Duration.ofSeconds(1);
...
regionFactory.setRegionTimeToLive(new
ExpirationAttributes(firstTtl.getSeconds(), DESTROY));
...
assertThat(Duration.between(startInstant, endInstant))
.isLessThan(firstTtlSeconds);
```
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7688: GEODE-10326: Convert MessageType to enum.
nabarunnag commented on code in PR #7688:
URL: https://github.com/apache/geode/pull/7688#discussion_r893815867
##
geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ClientUpdateMessageImpl.java:
##
@@ -1011,7 +1015,7 @@ public void sendTo(DataOutput out) throws IOException {
InternalDataSerializer.writeArrayLength(size, out);
if (size > 0) {
DataSerializer.writeObject(name[0], out);
-DataSerializer.writeObject(op, out);
+DataSerializer.writeObject(op.id, out);
Review Comment:
Is there a possibility of a NPE here or other places if we call members of
op, if we created CqNameToOpSingleEntry with op value as null. As we can see is
possible in by the new code change in HAEventWrapper.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] jinmeiliao merged pull request #7760: GEODE-10351: Wait till the cache is completely closed before re-creating
jinmeiliao merged PR #7760: URL: https://github.com/apache/geode/pull/7760 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7779: GEODE-6504: Instant class used.
nabarunnag commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893746594
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
- I would prefer to see in this test that it is greater or equal to the
increased TTL. (lets try this, stress tests were ok, if it fails again I will
make the change)
- Using the Duration comparison
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7779: GEODE-6504: Instant class used.
pivotal-jbarrett commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893607561
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
It sounds to me that it is more of a math problem, not a time resolution
problem. While `Instant` is meant for very precise instance in time, typically
for wall time, the `now()` construction of time is not very precise and uses
the wall clock from `System.currentTimeMillis()`. Using a lower resolution
timer may reduce then probably of the unit conversion floor but doesn't
eliminate it.
This assertion still fails:
```java
assertThat(TimeUnit.MILLISECONDS.toSeconds(7999L)).isEqualTo(8);
```
The problem comes form the conversion of time from higher resolution
duration to lower resolution duration and the expectation that this operation
happens at exactly the same time resolution as the measurement of the passage
of time. Compounding the change is that use of wall time from `Instant` could
actually be skewed by clock adjustments, where as `nanoTime` is a relatively
"fixed clock"*.
Basically, we can't say that this operation happened at precisely
`8.0s` or even `8.000s`. We could assert that it took place
approximately 8s with some tolerance. If we know that the event timer of the
operation we are measuring has a resolution finer than 1s, and that an event
never fires early, then we could assert that it took at least 7s.
My guess is, without digging Ito the internals, that the event time
resolution is 1ms based on `System.currentTimeMillis()` so the use of `Instant`
will "work" because they both use the same clock. My concern with the use of
`Instant` is more a matter of consistency with the rest of our tests and that
the source clock is somewhat obscured. If we are going to use `Instant` and
`Duration`, then we should probably use the `assertThat()` assertions for
`Duration`, like `closeTo()`.
this:
```java
assertThat(Duration.between(startInstant, endInstant))
.isGreaterThanOrEqualTo(Duration.ofSeconds(secondTtlSeconds));
```
or this:
```java
assertThat(Duration.between(startInstant, endInstant))
.isCloseTo(Duration.ofSeconds(secondTtlSeconds), Duration.ofSeconds(1));
```
_* `nanoTime()` has issues with different observers seeing different values
for the current tick leading some observations that time has either stopped or
gone backwards._
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] mkevo merged pull request #7629: GEODE-7875: fix create index gfsh command on partitioned region
mkevo merged PR #7629: URL: https://github.com/apache/geode/pull/7629 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag merged pull request #7787: Revert "GEODE-9632: Allow INDEX_THRESHOLD_SIZE System property to ove…
nabarunnag merged PR #7787: URL: https://github.com/apache/geode/pull/7787 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7779: GEODE-6504: Instant class used.
nabarunnag commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893014322
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
Instant class, newly introduced in Java 8. If it is the same as
currentTimeMillis(), should not matter which one we use right?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7779: GEODE-6504: Instant class used.
nabarunnag commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893017368
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
In the history of the this test failure if it always fails with: expected 8
sec and actual 7 sec
even if the nanoTIme is off by 1ns the difference is 79 which when
is passed to NANOSECOND.toSecond() just divides it by 10 which is 7 sec
and the test fails.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7779: GEODE-6504: Instant class used.
nabarunnag commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893014322
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
Instant class, newly introduced in Java 8. If both are the same, should not
matter which one we use right?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on a diff in pull request #7779: GEODE-6504: Instant class used.
nabarunnag commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r893011483
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
its in the javadocs and lot of blogs:
> Another pitfall with nanoTime() is that even though it provides nanosecond
precision, it doesn't guarantee nanosecond resolution (i.e. how often the value
is updated).
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7779: GEODE-6504: Instant class used.
pivotal-jbarrett commented on code in PR #7779:
URL: https://github.com/apache/geode/pull/7779#discussion_r892974555
##
geode-core/src/integrationTest/java/org/apache/geode/cache/RegionExpirationIntegrationTest.java:
##
@@ -70,10 +71,10 @@ public void setUp() {
}
@Test
- public void increaseRegionTtl() throws Exception {
+ public void increaseRegionTtl() {
int firstTtlSeconds = 3;
int secondTtlSeconds = 8;
-long startNanos = nanoTime();
+Instant startInstant = Instant.now();
Review Comment:
I don't understand the change based on the commit message. How is
`nanoTime()` a lower resolution clock that `currentTimeMilis()`, used to
construct this `Instant`?
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] nabarunnag commented on pull request #7787: Revert "GEODE-9632: Allow INDEX_THRESHOLD_SIZE System property to ove…
nabarunnag commented on PR #7787: URL: https://github.com/apache/geode/pull/7787#issuecomment-1150466453 @mkevo as mentioned in the dev mail, we are planning to put the commit in develop again after fixing the problem that was brought up by you in dev list. We are planning to release 1.15 (which is planned for this month) and then try to fix the issue and put it back into develop. We need to revert the commit in develop to back port the revert to 1.15 ( its the release protocol). After 1.15.0 is released, we can put the complete solution back in -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] demery-pivotal merged pull request #7772: GEODE-10321: Acceptance test for Geode access to JDK internals
demery-pivotal merged PR #7772: URL: https://github.com/apache/geode/pull/7772 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7789: GEODE-10342: Simplify copying jars
davebarnes97 merged PR #7789: URL: https://github.com/apache/geode/pull/7789 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7790: GEODE-10342: Simplify copying jars
davebarnes97 merged PR #7790: URL: https://github.com/apache/geode/pull/7790 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7788: GEODE-10342: Simplify copying jars
davebarnes97 merged PR #7788: URL: https://github.com/apache/geode/pull/7788 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7790: GEODE-10342: Simplify copying jars
animatedmax opened a new pull request, #7790: URL: https://github.com/apache/geode/pull/7790 Port of PR #7778 to support/1.12 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7789: GEODE-10342: Simplify copying jars
animatedmax opened a new pull request, #7789: URL: https://github.com/apache/geode/pull/7789 Port of PR #7778 to support/1.14 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7788: GEODE-10342: Simplify copying jars
animatedmax opened a new pull request, #7788: URL: https://github.com/apache/geode/pull/7788 Port of PR #7778 to support/1.15 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag opened a new pull request, #7787: Revert "GEODE-9632: Allow INDEX_THRESHOLD_SIZE System property to ove…
nabarunnag opened a new pull request, #7787: URL: https://github.com/apache/geode/pull/7787 …rride CompiledValue.RESULT_LIMIT (#7010)" This reverts commit 67359dcd ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag opened a new pull request, #7786: GEODE-10370: More info added to javadocs
nabarunnag opened a new pull request, #7786: URL: https://github.com/apache/geode/pull/7786 * Added what is returned by methods showJVMMetrics and showOSMetrics * Informtation include individual beans and what data type it is. ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] demery-pivotal commented on a diff in pull request #7772: GEODE-10321: Acceptance test for Geode access to JDK internals
demery-pivotal commented on code in PR #7772:
URL: https://github.com/apache/geode/pull/7772#discussion_r892721359
##
geode-assembly/src/acceptanceTest/java/org/apache/geode/jdk/JdkEncapsulationTest.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.geode.jdk;
+
+import static java.util.stream.Collectors.joining;
+import static
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
+import static org.apache.geode.test.util.JarUtils.createJarWithClasses;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.Assumptions.assumeThat;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.stream.Stream;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.test.junit.rules.FolderRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshExecution;
+import org.apache.geode.test.junit.rules.gfsh.GfshRule;
+import org.apache.geode.test.junit.rules.gfsh.GfshScript;
+import org.apache.geode.test.version.JavaVersions;
+
+/**
+ * Test several ways to make normally inaccessible JDK packages accessible on
JDK 17.
+ */
+public class JdkEncapsulationTest {
+ private static final String TRAVERSE_INACCESSIBLE_OBJECT = String.join(" ",
+ "execute", "function", "--id=" + TraverseInaccessibleJdkObject.ID);
+
+ private static final Path LINUX_ARGUMENT_FILE_RELATIVE_PATH =
+ Paths.get(System.getenv("GEODE_HOME"), "config",
"open-all-jdk-packages-linux-openjdk-17")
+ .toAbsolutePath().normalize();
+
+ @Rule(order = 0)
+ public final FolderRule folderRule = new FolderRule();
+
+ @Rule(order = 1)
+ public final GfshRule gfshRule = new GfshRule(folderRule::getFolder);
+
+ private String connectToLocator;
+ private int serverPort;
+ private String locatorString;
+
+ @BeforeClass
+ public static void validOnlyOnJdk17AndLater() {
+assumeThat(JavaVersions.current().specificationVersion())
+.isGreaterThanOrEqualTo(17);
+ }
+
+ @Before
+ public void startLocatorWithObjectTraverserFunction() throws IOException {
+int[] availablePorts = getRandomAvailableTCPPorts(2);
+int locatorPort = availablePorts[0];
+serverPort = availablePorts[1];
+locatorString = "localhost[" + locatorPort + "]";
+connectToLocator = "connect --locator=" + locatorString;
+
+String startLocator = String.join(" ",
+"start",
+"locator",
+"--port=" + locatorPort,
+"--http-service-port=0");
+
+Path jarPath =
folderRule.getFolder().toPath().resolve("traverse-object.jar");
+createJarWithClasses(jarPath, TraverseInaccessibleJdkObject.class);
+String deployObjectTraverserFunction = String.join(" ",
+"deploy",
+"--jar=" + jarPath);
+gfshRule.execute(startLocator, deployObjectTraverserFunction);
+ }
+
+ // If this test fails, it means the object we're trying to traverse has no
inaccessible fields,
+ // and so is not useful for the other tests. If it fails, update
TraverseInaccessibleJdkObject
+ // to use a type that actually has inaccessible fields.
+ @Test
+ public void cannotMakeInaccessibleFieldsAccessibleByDefault() {
+gfshRule.execute(startServerWithOptions()); // No options
+
+GfshExecution execution = GfshScript.of(connectToLocator)
+.and(TRAVERSE_INACCESSIBLE_OBJECT)
+.expectExitCode(1) // Because java.math is not opened by default.
+.execute(gfshRule);
+
+assertThat(execution.getOutputText())
+.as("result of traversing %s",
TraverseInaccessibleJdkObject.OBJECT.getClass())
+.contains("Exception: java.lang.reflect.InaccessibleObjectException");
+ }
+
+ @Test
+ public void canMakeFieldsAccessibleOnExplicitlyOpenedPackages() {
+String opensOptionFormat = "--J=--add-opens=%s/%s=ALL-UNNAMED";
+String objectPackage =
TraverseInaccessibleJdkObject.OBJECT.getClass().getPackage().getName();
+String objectModule = TraverseInaccessibleJdkObject.MODULE;
+String opensOption = String.format(opensOptionFormat, objectModule,
objectPackage);
+
[GitHub] [geode] Bill opened a new pull request, #7785: [DRAFT] delay sync after member departure by 60s in P2P-only config
Bill opened a new pull request, #7785: URL: https://github.com/apache/geode/pull/7785 In a cluster configured for P2P-only (no clients possible—no CacheServer instance present), if a primary bucket holder crashes while servicing a partitioned.PutMessage (in this instance, the result of a Region.create(K key, V value) call on some peer) before distributing UpdateOperations to all peers, if a peer that did not receive an UpdateOperation is chosen as the new primary, the subsequent retry of the create/put on the new primary can result in an EntryExistsException. The root cause is that when peers learn of member departure, if the cluster is configured for P2P-only operation (no clients), the members will immediately initiate delta GII to acquire the departed member’s data (from remaining members). But this synchronization does not include events. Because events are not included, the new primary can have the newly-created entry and no corresponding event (with which to conflate a subsequent create request). ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz merged pull request #7424: GEODE-10068: Make WanCopyRegionFunctionService thread pool configurab…
albertogpz merged PR #7424: URL: https://github.com/apache/geode/pull/7424 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] albertogpz opened a new pull request, #7784: [DRAFT DO NOT REVIEW] Wip/handshake locks
albertogpz opened a new pull request, #7784: URL: https://github.com/apache/geode/pull/7784 ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] mkevo commented on pull request #7643: GEODE-10267: fix creating gw sender with non-existent disk store
mkevo commented on PR #7643: URL: https://github.com/apache/geode/pull/7643#issuecomment-1149637793 Hi @gesterzhou, @boglesby, @nabarunnag, @pivotal-jbarrett, can you please add your review on this? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal merged pull request #7783: GEODE-10089: update LICENSE
onichols-pivotal merged PR #7783: URL: https://github.com/apache/geode/pull/7783 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] onichols-pivotal opened a new pull request, #7783: GEODE-10089: update LICENSE
onichols-pivotal opened a new pull request, #7783: URL: https://github.com/apache/geode/pull/7783 we have some dependency changes due to recent redis removal, change to springdoc and inclusion of joda-time -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a diff in pull request #7665: GEODE-10281: Fix WAN data inconsistency
pivotal-jbarrett commented on code in PR #7665:
URL: https://github.com/apache/geode/pull/7665#discussion_r891801945
##
geode-wan/src/distributedTest/java/org/apache/geode/internal/cache/wan/serial/TestCacheWriterDelayWritingOfEntry.java:
##
@@ -0,0 +1,82 @@
+/*
+ * 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.geode.internal.cache.wan.serial;
+
+import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Map;
+
+import org.apache.geode.cache.CacheWriter;
+import org.apache.geode.cache.CacheWriterException;
+import org.apache.geode.cache.EntryEvent;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionEvent;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+
+public class TestCacheWriterDelayWritingOfEntry implements
CacheWriter {
Review Comment:
nit: If this test class is only going to be used by the single test suite
then perhaps it should be nested type rather than a top level.
##
geode-core/src/test/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImplTest.java:
##
@@ -377,6 +377,37 @@ public void
testCreation_WithAfterUpdateWithGenerateCallbacks(boolean isGenerate
assertThat(event.getAction()).isEqualTo(action);
}
+ @Test
Review Comment:
Convert this unit test to use JUnit 5.
--
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: notifications-unsubscr...@geode.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7782: GEODE-10365: add missing components to table
davebarnes97 merged PR #7782: URL: https://github.com/apache/geode/pull/7782 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7781: GEODE-10365: add missing components to table
davebarnes97 merged PR #7781: URL: https://github.com/apache/geode/pull/7781 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7780: GEODE-10365: add missing components to table
davebarnes97 merged PR #7780: URL: https://github.com/apache/geode/pull/7780 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] davebarnes97 merged pull request #7778: GEODE-10342: Simplify copying jars
davebarnes97 merged PR #7778: URL: https://github.com/apache/geode/pull/7778 -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7782: GEODE-10365: add missing components to table
animatedmax opened a new pull request, #7782: URL: https://github.com/apache/geode/pull/7782 Port of PR #7775 to support/1.12 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7781: GEODE-10365: add missing components to table
animatedmax opened a new pull request, #7781: URL: https://github.com/apache/geode/pull/7781 Port of PR #7775 to support/1.14 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] animatedmax opened a new pull request, #7780: GEODE-10365: add missing components to table
animatedmax opened a new pull request, #7780: URL: https://github.com/apache/geode/pull/7780 Port of PR #7775 to support/1.15 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] nabarunnag opened a new pull request, #7779: GEODE-6504: Instant class used.
nabarunnag opened a new pull request, #7779: URL: https://github.com/apache/geode/pull/7779 * Using instant class instead of nanos. * Nanos is not updated that often hence not proper resolution. * Using millis instead of seconds to avoid ceiling issues. ### For all changes: - [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? - [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)? - [ ] Is your initial contribution a single, squashed commit? - [ ] Does `gradlew build` run cleanly? - [ ] Have you written or updated unit tests to verify your changes? - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [geode] jinmeiliao opened a new pull request, #7777: GEODE-6489: fix a flaky test
jinmeiliao opened a new pull request, #: URL: https://github.com/apache/geode/pull/ * fix GemFireDeadlockDetectorDUnitTest.testDistributedDeadlockWithDLock -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
