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.
---
