[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...
GitHub user poornachandra opened a pull request: https://github.com/apache/incubator-tephra/pull/74 TEPHRA-266 Identify log messages when multiple instances of Tephra run on a single HBase cluster JIRA - https://issues.apache.org/jira/browse/TEPHRA-266 You can merge this pull request into a Git repository by running: $ git pull https://github.com/poornachandra/incubator-tephra feature/tx-state-cache-log Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tephra/pull/74.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #74 commit df16b5be2146c4ae92d23d2c62a2cf3b32a0ced5 Author: poorna Date: 2018-04-26T01:58:52Z TEPHRA-266 Identify log messages when multiple instances of Tephra run on a single HBase cluster ---
[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/74#discussion_r184322363
--- Diff:
tephra-core/src/main/java/org/apache/tephra/coprocessor/TransactionStateCache.java
---
@@ -184,4 +185,14 @@ private void refreshState() throws IOException {
public TransactionVisibilityState getLatestState() {
return latestState;
}
+
+ protected void setId(@Nullable String id) {
+if (id != null) {
+ this.logPrefix = "[" + id + "] ";
+}
+ }
+
+ private String prefixLog(String message) {
--- End diff --
not sure this is a very good idea. It means you are performing the string
operations even when it is not being logged (for example, for debug messages).
Better to add the logPrefix as an argument to the log message, such as:
```
LOG.debug("[{}] Latest transaction snapshot: {}", logPrefix,
latestState.toString()));
```
---
[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...
Github user poornachandra commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/74#discussion_r185070466
--- Diff:
tephra-core/src/main/java/org/apache/tephra/coprocessor/TransactionStateCache.java
---
@@ -184,4 +185,14 @@ private void refreshState() throws IOException {
public TransactionVisibilityState getLatestState() {
return latestState;
}
+
+ protected void setId(@Nullable String id) {
+if (id != null) {
+ this.logPrefix = "[" + id + "] ";
+}
+ }
+
+ private String prefixLog(String message) {
--- End diff --
HBase co-processor uses Apache commons-logging. The `{}` notation is not
available in the commons-logging APIs.
I couldn't figure out a way to not create a string unless absolutely
necessary. The only debug log in that file has `LOG.isDebugEnabled()` guard.
However, I know this is not an ideal solution.
---
[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...
Github user anew commented on a diff in the pull request:
https://github.com/apache/incubator-tephra/pull/74#discussion_r185305932
--- Diff:
tephra-core/src/main/java/org/apache/tephra/coprocessor/TransactionStateCache.java
---
@@ -184,4 +185,14 @@ private void refreshState() throws IOException {
public TransactionVisibilityState getLatestState() {
return latestState;
}
+
+ protected void setId(@Nullable String id) {
+if (id != null) {
+ this.logPrefix = "[" + id + "] ";
+}
+ }
+
+ private String prefixLog(String message) {
--- End diff --
I was not aware of that. I think this is not logged very frequently, so
it's ok.
---
[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...
Github user poornachandra closed the pull request at: https://github.com/apache/incubator-tephra/pull/74 ---
