tephra-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anew <...@git.apache.org>
Subject [GitHub] incubator-tephra pull request #53: (TEPHRA-243) Improve logging for slow log...
Date Wed, 13 Sep 2017 17:38:10 GMT
Github user anew commented on a diff in the pull request:

    https://github.com/apache/incubator-tephra/pull/53#discussion_r138688512
  
    --- Diff: tephra-core/src/main/java/org/apache/tephra/persist/AbstractTransactionLog.java
---
    @@ -165,26 +203,36 @@ private void sync() throws IOException {
           // prevent writer being dereferenced
           tmpWriter = writer;
     
    -      List<Entry> currentPending = getPendingWrites();
    -      if (!currentPending.isEmpty()) {
    -        tmpWriter.commitMarker(currentPending.size());
    -      }
    -
    -      // write out all accumulated entries to log.
    -      for (Entry e : currentPending) {
    -        tmpWriter.append(e);
    -        entryCount++;
    -        latestSeq = Math.max(latestSeq, e.getKey().get());
    +      Entry[] currentPending = getPendingWrites();
    +      if (currentPending != null) {
    +        entryCount = currentPending.length;
    +        startTimerIfNeeded(tmpWriter, entryCount);
    +        tmpWriter.commitMarker(entryCount);
    +        for (Entry e : currentPending) {
    +          tmpWriter.append(e);
    +          latestSeq = Math.max(latestSeq, e.getKey().get());
    +        }
    +        writtenUpTo.set(latestSeq);
    --- End diff --
    
    I was thinking about that. However, it would add latency. In the following case: Suppose:
    - thread 1 writes up to seq id 5
    - thread 2 writes up to seq id 10
    - thread 2 syncs and sets syncedUpto to 10
    - thread 3 writes up to seq id 15
    - now thread 1 checks whether it needs to sync. Because its latestSeq is 5, it can return.
But if it compares with writtenUpto, then it would try to sync. 
    
    That would add latency to thread 1 while thread 3 must wait anyway. That's why I introduced
the writtenUpto
    
    By the way, the existing code had syncedUpto wrong. It was always set to latestSeq of
the thread that syncs. But it might have synced more than its own entries. For example, in
the scenario above, if thread 1 syncs instead of thread 2, then it has synced everything up
to 10, but it would set syncedUpto to 5. With the consequence that thread would sync again,
needlessly.


---

Mime
View raw message