You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2022/02/16 22:20:57 UTC
[GitHub] [geode] nabarunnag opened a new pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
nabarunnag opened a new pull request #7375:
URL: https://github.com/apache/geode/pull/7375
* Logging calls with number of placeholder not equal to the number of
arguments were fixed in this PR.
<!-- Thank you for submitting a contribution to Apache Geode. -->
<!-- In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
-->
### 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)?
<!-- Note:
Please ensure that once the PR is submitted, check Concourse for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
-->
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808515455
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -1257,7 +1257,7 @@ boolean canAccommodateMoreBytesSafely(int bytes) {
final long curBytes = bytesInUse.get();
if (isDebugEnabled) {
logger.debug(
- "canAccomodateMoreBytes: bytes = {} allocatedMemory = {} newAllocatedSize = {} thresholdSize = ",
+ "canAccomodateMoreBytes: bytes ={} allocatedMemory ={} newAllocatedSize ={} thresholdSize ={}",
Review comment:
was trying to keep the spacing unform in a way. There are no spaces in ':{}' (sometimes they do) hence was trying to keep things uniform.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808519504
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/LoginTimeOutDUnitTest.java
##########
@@ -263,15 +263,15 @@ public static void runTest1() throws Exception {
logger.debug("Naming Exception caught in lookup: " + e);
fail("failed in naming lookup: " + e);
} catch (Exception e) {
- logger.debug("Exception caught during naming lookup: {}", e);
+ logger.debug("Exception caught during naming lookup:", e);
fail("failed in naming lookup: " + e);
}
try {
for (int count = 0; count < MAX_CONNECTIONS; count++) {
ds.getConnection();
}
} catch (Exception e) {
- logger.debug("Exception caught in runTest1: {}", e);
+ logger.debug("Exception caught in runTest1:", e);
Review comment:
debug logs, I think the whole stack is better than the message
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808515918
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueStateImpl.java
##########
@@ -310,7 +310,7 @@ void sendPeriodicAck() {
success = true;
} catch (Exception ex) {
if (logger.isDebugEnabled()) {
- logger.debug("Exception while sending an ack to the primary server: {}", ex);
+ logger.debug("Exception while sending an ack to the primary server:", ex);
Review comment:
It's debug enabled, hence the entire stack is always better than just the message
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808519973
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/LoginTimeOutDUnitTest.java
##########
@@ -263,15 +263,15 @@ public static void runTest1() throws Exception {
logger.debug("Naming Exception caught in lookup: " + e);
fail("failed in naming lookup: " + e);
} catch (Exception e) {
- logger.debug("Exception caught during naming lookup: {}", e);
+ logger.debug("Exception caught during naming lookup:", e);
fail("failed in naming lookup: " + e);
}
try {
for (int count = 0; count < MAX_CONNECTIONS; count++) {
ds.getConnection();
}
} catch (Exception e) {
- logger.debug("Exception caught in runTest1: {}", e);
+ logger.debug("Exception caught in runTest1:", e);
Review comment:
Also, its just a test, should not matter much
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808516062
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
##########
@@ -328,8 +328,8 @@ protected void basicPerformTimeout(boolean isPending) throws CacheException {
if (logger.isTraceEnabled()) {
// NOTE: original finer message used this.toString() twice
Review comment:
will remove
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] pivotal-jbarrett commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
pivotal-jbarrett commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808510116
##########
File path: geode-core/src/main/java/org/apache/geode/cache/client/internal/QueueStateImpl.java
##########
@@ -310,7 +310,7 @@ void sendPeriodicAck() {
success = true;
} catch (Exception ex) {
if (logger.isDebugEnabled()) {
- logger.debug("Exception while sending an ack to the primary server: {}", ex);
+ logger.debug("Exception while sending an ack to the primary server:", ex);
Review comment:
Is it possible that this production code intends to just log the message and not the entire stack trace?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
##########
@@ -328,8 +328,8 @@ protected void basicPerformTimeout(boolean isPending) throws CacheException {
if (logger.isTraceEnabled()) {
// NOTE: original finer message used this.toString() twice
Review comment:
I don't this comment is necessary anymore.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLessorDepartureHandler.java
##########
@@ -108,8 +108,8 @@ private void sendRecoveryMsgs(final DistributionManager dm, final DLockBatch[] b
// this shouldn't happen unless we're shutting down or someone has set a size constraint
// on the waiting-pool using a system property
if (!dm.getCancelCriterion().isCancelInProgress()) {
- logger.warn("Unable to schedule background cleanup of transactions for departed member {}."
- + " Performing in-line cleanup of the transactions.");
+ logger.warn(
+ "Unable to schedule background cleanup of transactions for departed member. Performing in-line cleanup of the transactions.");
Review comment:
Is there a member to log here?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/execute/ServerToClientFunctionResultSender.java
##########
@@ -98,7 +98,7 @@ public synchronized void lastResult(Object oneResult) {
}
if (logger.isDebugEnabled()) {
- logger.debug("ServerToClientFunctionResultSender sending last result1 {} " + oneResult);
+ logger.debug("ServerToClientFunctionResultSender sending last result1 {} ", oneResult);
Review comment:
Trailing whitespace.
##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/jta/dunit/LoginTimeOutDUnitTest.java
##########
@@ -263,15 +263,15 @@ public static void runTest1() throws Exception {
logger.debug("Naming Exception caught in lookup: " + e);
fail("failed in naming lookup: " + e);
} catch (Exception e) {
- logger.debug("Exception caught during naming lookup: {}", e);
+ logger.debug("Exception caught during naming lookup:", e);
fail("failed in naming lookup: " + e);
}
try {
for (int count = 0; count < MAX_CONNECTIONS; count++) {
ds.getConnection();
}
} catch (Exception e) {
- logger.debug("Exception caught in runTest1: {}", e);
+ logger.debug("Exception caught in runTest1:", e);
Review comment:
Is it possible that these log messages intended to log the message only and not the stacktrace?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -1257,7 +1257,7 @@ boolean canAccommodateMoreBytesSafely(int bytes) {
final long curBytes = bytesInUse.get();
if (isDebugEnabled) {
logger.debug(
- "canAccomodateMoreBytes: bytes = {} allocatedMemory = {} newAllocatedSize = {} thresholdSize = ",
+ "canAccomodateMoreBytes: bytes ={} allocatedMemory ={} newAllocatedSize ={} thresholdSize ={}",
Review comment:
Was it intentional to remove the whitespace here?
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/UpdateAttributesProcessor.java
##########
@@ -462,7 +462,7 @@ public static void send(InternalDistributedMember recipient, int processorId,
if (exception != null) {
m.setException(exception);
if (logger.isDebugEnabled()) {
- logger.debug("Replying with exception: {}" + m, exception);
+ logger.debug("Replying with exception:{}", m, exception);
Review comment:
I wonder if the intent was for just the exception message here and not the stack.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -3022,7 +3022,7 @@ public void handleInterestEvent(InterestRegistrationEvent event) {
// If this is a registration event, add interest for this key
if (isRegister) {
if (logger.isDebugEnabled()) {
- logger.debug("PartitionedRegionDataStore for {} adding interest for: ",
+ logger.debug("PartitionedRegionDataStore for {} adding interest for :{}",
Review comment:
Whitespace change seems wrong.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808533828
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/locks/TXLessorDepartureHandler.java
##########
@@ -108,8 +108,8 @@ private void sendRecoveryMsgs(final DistributionManager dm, final DLockBatch[] b
// this shouldn't happen unless we're shutting down or someone has set a size constraint
// on the waiting-pool using a system property
if (!dm.getCancelCriterion().isCancelInProgress()) {
- logger.warn("Unable to schedule background cleanup of transactions for departed member {}."
- + " Performing in-line cleanup of the transactions.");
+ logger.warn(
+ "Unable to schedule background cleanup of transactions for departed member. Performing in-line cleanup of the transactions.");
Review comment:
found it. adding it to the log
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808517944
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -3022,7 +3022,7 @@ public void handleInterestEvent(InterestRegistrationEvent event) {
// If this is a registration event, add interest for this key
if (isRegister) {
if (logger.isDebugEnabled()) {
- logger.debug("PartitionedRegionDataStore for {} adding interest for: ",
+ logger.debug("PartitionedRegionDataStore for {} adding interest for :{}",
Review comment:
if we search for :{} in the project, we see that it is being used. I remember all logs I analyzed in hydra we used this format. Helps in easier greps
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808521660
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/execute/ServerToClientFunctionResultSender.java
##########
@@ -98,7 +98,7 @@ public synchronized void lastResult(Object oneResult) {
}
if (logger.isDebugEnabled()) {
- logger.debug("ServerToClientFunctionResultSender sending last result1 {} " + oneResult);
+ logger.debug("ServerToClientFunctionResultSender sending last result1 {} ", oneResult);
Review comment:
will fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] dschneider-pivotal commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
dschneider-pivotal commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r814987051
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Size.java
##########
@@ -72,7 +72,7 @@ public void cmdExecute(final @NotNull Message clientMessage,
String regionName = regionNamePart.getCachedString();
if (regionName == null) {
- logger.warn("The input region name for the %s request is null", "size");
+ logger.warn("The input region name for the size request is null");
Review comment:
while you are in this part of the code consider fixing the errMessage.append call also. It seems like we should just have a local String constant that is in both calls.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -5126,8 +5126,8 @@ public static void validatePRID(InternalDistributedMember sender, int prId, Stri
PartitionedRegion pr = (PartitionedRegion) o;
if (pr.getPRId() == prId) {
if (!pr.getRegionIdentifier().equals(regionId)) {
- logger.warn("{} is using PRID {} for {} but this process maps that PRID to {}",
- new Object[] {sender.toString(), prId, pr.getRegionIdentifier()});
+ logger.warn("{} is using PRID {} for regionId {} but this process maps that PRID to {}",
Review comment:
could "sender.toString()" just be "sender"? This will improve performance if warnings are disabled (which is unlikely).
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -5613,7 +5613,7 @@ public void cleanupFailedInitialization() {
}
if (savedFirstRuntimeException != null
&& savedFirstRuntimeException instanceof DistributedSystemDisconnectedException) {
- logger.warn("cleanupFailedInitialization originally failed with {}",
+ logger.warn("cleanupFailedInitialization originally failed with: ",
Review comment:
Did the old code for this warning log the call stack for savedFirstRuntimeException? It looks like the intent was just to log savedFirstRuntimeException.toString() inside the curly brackets.
But if the old behavior was to log the stack then I think your change is good.
All the places we consider setting savedFirstRuntimeException we also log a warning with the full stack. So I think that intent of this message was just to say "cleanup actually did fail and here is the exception of all the ones we already logged that caused the failure". It seems odd to me that these are warnings. I'm not sure what a customer could do with these warnings.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r808521094
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/UpdateAttributesProcessor.java
##########
@@ -462,7 +462,7 @@ public static void send(InternalDistributedMember recipient, int processorId,
if (exception != null) {
m.setException(exception);
if (logger.isDebugEnabled()) {
- logger.debug("Replying with exception: {}" + m, exception);
+ logger.debug("Replying with exception:{}", m, exception);
Review comment:
I think it is safe as it is inside debug flag
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] nabarunnag commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
nabarunnag commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r816130357
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/Size.java
##########
@@ -72,7 +72,7 @@ public void cmdExecute(final @NotNull Message clientMessage,
String regionName = regionNamePart.getCachedString();
if (regionName == null) {
- logger.warn("The input region name for the %s request is null", "size");
+ logger.warn("The input region name for the size request is null");
Review comment:
Done with https://github.com/apache/geode/pull/7399
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] onichols-pivotal commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
onichols-pivotal commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r813492949
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -1257,7 +1257,7 @@ boolean canAccommodateMoreBytesSafely(int bytes) {
final long curBytes = bytesInUse.get();
if (isDebugEnabled) {
logger.debug(
- "canAccomodateMoreBytes: bytes = {} allocatedMemory = {} newAllocatedSize = {} thresholdSize = ",
+ "canAccomodateMoreBytes: bytes ={} allocatedMemory ={} newAllocatedSize ={} thresholdSize ={}",
Review comment:
If removing whitespace, `x=5 y=7` seems more natural than `x =5 y =7` 🤷♂️
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] boglesby commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
boglesby commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r819745386
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegionDataStore.java
##########
@@ -2953,7 +2953,7 @@ public void executeOnDataStore(final Set localKeys, final Function function, fin
long start = stats.startFunctionExecution(function.hasResult());
try {
if (logger.isDebugEnabled()) {
- logger.debug("Executing Function: {} on Remote Node with context: ", function.getId(),
+ logger.debug("Executing Function:{} on Remote Node with context: {}", function.getId(),
Review comment:
The space was removed in `Function:{}`. It probably should also be removed in `context: {}`
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java
##########
@@ -5613,7 +5613,7 @@ public void cleanupFailedInitialization() {
}
if (savedFirstRuntimeException != null
&& savedFirstRuntimeException instanceof DistributedSystemDisconnectedException) {
- logger.warn("cleanupFailedInitialization originally failed with {}",
+ logger.warn("cleanupFailedInitialization originally failed with: ",
Review comment:
I like having the stack on warnings.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/BucketRegionQueue.java
##########
@@ -146,7 +146,7 @@ private void initializeEventSeqNumQueue() {
if (logger.isDebugEnabled()) {
logger.debug(
- "For bucket {} ,total keys recovered are : {} last key recovered is : {} and the seqNo is ",
+ "For bucket: {} ,total keys recovered are: {} last key recovered is: {} and the seqNo is: {}",
Review comment:
This: `{} ,` should be this: `{}, `. Also there are no commas in the rest of that message, so maybe the comma should be removed altogether.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jchen21 commented on a change in pull request #7375: GEODE-6588: Fixed mismatch of placeholders and arguments
Posted by GitBox <gi...@apache.org>.
jchen21 commented on a change in pull request #7375:
URL: https://github.com/apache/geode/pull/7375#discussion_r819790167
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/UpdateAttributesProcessor.java
##########
@@ -393,7 +393,7 @@ public static void send(InternalDistributedMember recipient, int processorId,
if (exception != null) {
m.setException(exception);
if (logger.isDebugEnabled()) {
- logger.debug("Replying with exception: {}" + m, exception);
+ logger.debug("Replying with exception: {}", m, exception);
Review comment:
Missing `{}` here.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/UpdateAttributesProcessor.java
##########
@@ -462,7 +462,7 @@ public static void send(InternalDistributedMember recipient, int processorId,
if (exception != null) {
m.setException(exception);
if (logger.isDebugEnabled()) {
- logger.debug("Replying with exception: {}" + m, exception);
+ logger.debug("Replying with exception: {}", m, exception);
Review comment:
Missing `{}` here.
##########
File path: geode-core/src/main/java/org/apache/geode/internal/cache/EntryExpiryTask.java
##########
@@ -326,10 +326,9 @@ protected void basicPerformTimeout(boolean isPending) throws CacheException {
}
if (hasExpired(getNow(), expTime)) {
if (logger.isTraceEnabled()) {
- // NOTE: original finer message used this.toString() twice
logger.trace(
- "{}.performTimeout().getExpirationTime() is {}; {}.expire({}). ttlExpiration: {}, idleExpiration: {}, ttlAttrs: {}, idleAttrs: {} action is: {}",
- this, expTime, this, action, ttl, idle, getTTLAttributes(), getIdleAttributes());
+ "{}.performTimeout().getExpirationTime() is {}; ttlExpiration:{}, idleExpiration:{}, ttlAttrs:{}, idleAttrs:{} action is:{}",
Review comment:
`this` is passed as parameter for `{}.performTimeout().getExpirationTime()`. It will call `EntryExpiryTask.toString()` which returns a long string description with spaces. It is not easy to read. Perhaps passing `this.getClass().getSimpleName()` would be better. And the method name should be `basicPerformTimeout` instead of `performTimeout()`.
--
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-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org