You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tephra.apache.org by poornachandra <gi...@git.apache.org> on 2018/04/26 02:02:40 UTC

[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 <po...@...>
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...

Posted by anew <gi...@git.apache.org>.
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...

Posted by poornachandra <gi...@git.apache.org>.
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 issue #74: TEPHRA-266 Identify log messages when multiple i...

Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra commented on the issue:

    https://github.com/apache/incubator-tephra/pull/74
  
    This has been merged to master branch


---

[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...

Posted by poornachandra <gi...@git.apache.org>.
Github user poornachandra closed the pull request at:

    https://github.com/apache/incubator-tephra/pull/74


---

[GitHub] incubator-tephra pull request #74: TEPHRA-266 Identify log messages when mul...

Posted by anew <gi...@git.apache.org>.
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()));
    ``` 



---