You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2019/03/15 13:35:09 UTC
svn commit: r1855590 - in /jackrabbit/oak/trunk/oak-segment-tar/src:
main/java/org/apache/jackrabbit/oak/segment/
main/java/org/apache/jackrabbit/oak/segment/scheduler/
test/java/org/apache/jackrabbit/oak/segment/
Author: mduerig
Date: Fri Mar 15 13:35:08 2019
New Revision: 1855590
URL: http://svn.apache.org/viewvc?rev=1855590&view=rev
Log:
OAK-8094: JMX monitoring to detect commits carrying over from previous GC generation can block other threads from committing
Expose gc generation of commit
Modified:
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CommitsTracker.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreMonitor.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java
jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java
jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CommitsTrackerTest.java
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CommitsTracker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CommitsTracker.java?rev=1855590&r1=1855589&r2=1855590&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CommitsTracker.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/CommitsTracker.java Fri Mar 15 13:35:08 2019
@@ -29,9 +29,11 @@ import java.util.Map;
import java.util.Queue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
-import java.util.stream.Stream;
+import java.util.function.Supplier;
import com.googlecode.concurrentlinkedhashmap.ConcurrentLinkedHashMap;
+import org.apache.jackrabbit.oak.segment.file.tar.GCGeneration;
+import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
@@ -43,7 +45,11 @@ import org.jetbrains.annotations.Nullabl
* currently waiting on the commit semaphore
* </ul>
*
- * This class delegates thread-safety to its underlying state variables.
+ * For the most part, this class delegates thread-safety to its underlying
+ * state variables. However, the {@link #trackDequedCommitOf(Thread)} and
+ * {@link #trackExecutedCommitOf(Thread)} method must be called in
+ * sequence within the same transaction, because they are linked
+ * via the {@link #currentCommit} field.
*/
class CommitsTracker {
private final String[] threadGroups;
@@ -59,46 +65,54 @@ class CommitsTracker {
static final class Commit {
private final String threadName;
private final WeakReference<Thread> thread;
+ private final Supplier<GCGeneration> gcGeneration;
private long queued;
private long dequeued;
private long applied;
- Commit(Thread thread) {
+ Commit(Thread thread, Supplier<GCGeneration> gcGeneration) {
this.threadName = thread.getName();
+ this.gcGeneration = gcGeneration;
this.thread = new WeakReference<>(thread);
}
+ @NotNull
Commit queued() {
queued = System.currentTimeMillis();
return this;
}
+ @NotNull
Commit dequeued() {
dequeued = System.currentTimeMillis();
return this;
}
+ @NotNull
Commit applied() {
applied = System.currentTimeMillis();
return this;
}
- String getStackTrace() {
+ @Nullable
+ StackTraceElement[] getStackTrace() {
Thread t = thread.get();
- if (t != null) {
- StringBuilder threadDetails = new StringBuilder();
- Stream.of(t.getStackTrace()).forEach(threadDetails::append);
- return threadDetails.toString();
- } else {
- return "N/A";
- }
+ return t == null
+ ? null
+ : t.getStackTrace();
}
+ @NotNull
String getThreadName() {
return threadName;
}
+ @Nullable
+ GCGeneration getGCGeneration() {
+ return gcGeneration.get();
+ }
+
long getQueued() {
return queued;
}
@@ -118,8 +132,8 @@ class CommitsTracker {
this.queuedWritersMap = new ConcurrentHashMap<>();
}
- public void trackQueuedCommitOf(Thread thread) {
- queuedWritersMap.put(thread.getName(), new Commit(thread).queued());
+ public void trackQueuedCommitOf(Thread thread, Supplier<GCGeneration> gcGeneration) {
+ queuedWritersMap.put(thread.getName(), new Commit(thread, gcGeneration).queued());
}
public void trackDequedCommitOf(Thread thread) {
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreMonitor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreMonitor.java?rev=1855590&r1=1855589&r2=1855590&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreMonitor.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreMonitor.java Fri Mar 15 13:35:08 2019
@@ -19,6 +19,10 @@
package org.apache.jackrabbit.oak.segment;
+import java.util.function.Supplier;
+
+import org.apache.jackrabbit.oak.segment.file.tar.GCGeneration;
+
/**
* SegmentNodeStoreMonitor is notified for commit related operations performed by SegmentNodeStore.
*/
@@ -31,7 +35,7 @@ public interface SegmentNodeStoreMonitor
}
@Override
- public void onCommitQueued(Thread t) {
+ public void onCommitQueued(Thread t, Supplier<GCGeneration> gcGeneration) {
}
@@ -53,8 +57,9 @@ public interface SegmentNodeStoreMonitor
* queued for later retry.
*
* @param t the thread which initiated the write
+ * @param gcGeneration the commit's gc generation
*/
- void onCommitQueued(Thread t);
+ void onCommitQueued(Thread t, Supplier<GCGeneration> gcGeneration);
/**
* Notifies the monitor when a queued commit was dequeued for processing.
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java?rev=1855590&r1=1855589&r2=1855590&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/SegmentNodeStoreStats.java Fri Mar 15 13:35:08 2019
@@ -26,6 +26,8 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
import javax.management.openmbean.CompositeData;
import javax.management.openmbean.CompositeDataSupport;
@@ -40,6 +42,7 @@ import javax.management.openmbean.Tabula
import com.google.common.collect.ImmutableMap;
import org.apache.jackrabbit.api.stats.TimeSeries;
import org.apache.jackrabbit.oak.segment.CommitsTracker.Commit;
+import org.apache.jackrabbit.oak.segment.file.tar.GCGeneration;
import org.apache.jackrabbit.oak.stats.CounterStats;
import org.apache.jackrabbit.oak.stats.MeterStats;
import org.apache.jackrabbit.oak.stats.StatisticsProvider;
@@ -89,9 +92,9 @@ public class SegmentNodeStoreStats imple
}
@Override
- public void onCommitQueued(Thread t) {
+ public void onCommitQueued(Thread t, Supplier<GCGeneration> gcGeneration) {
commitQueueSize.inc();
- commitsTracker.trackQueuedCommitOf(t);
+ commitsTracker.trackQueuedCommitOf(t, gcGeneration);
}
@Override
@@ -163,9 +166,9 @@ public class SegmentNodeStoreStats imple
private static CompositeType getCompositeType(String name) throws OpenDataException {
return new CompositeType(
name, name,
- new String[] {"writerName", "writerDetails", "queued", "dequeued", "applied"},
- new String[] {"writerName", "writerDetails", "queued", "dequeued", "applied"},
- new OpenType[] {SimpleType.STRING, SimpleType.STRING,
+ new String[] {"writerName", "writerDetails", "GCGeneration", "queued", "dequeued", "applied"},
+ new String[] {"writerName", "writerDetails", "GCGeneration", "queued", "dequeued", "applied"},
+ new OpenType[] {SimpleType.STRING, SimpleType.STRING, SimpleType.STRING,
SimpleType.LONG, SimpleType.LONG, SimpleType.LONG});
}
@@ -199,12 +202,32 @@ public class SegmentNodeStoreStats imple
@NotNull
private Map<String, Object> toMap(@NotNull Commit commit) {
- return ImmutableMap.of(
- "writerName", commit.getThreadName(),
- "writerDetails", collectStackTraces ? commit.getStackTrace() : "N/A",
- "queued", commit.getQueued(),
- "dequeued", commit.getDequeued(),
- "applied", commit.getApplied());
+ return ImmutableMap.<String, Object>builder()
+ .put("writerName", commit.getThreadName())
+ .put("writerDetails", toString(commit.getStackTrace()))
+ .put("GCGeneration", toString(commit.getGCGeneration()))
+ .put("queued", commit.getQueued())
+ .put("dequeued", commit.getDequeued())
+ .put("applied", commit.getApplied())
+ .build();
+ }
+
+ @NotNull
+ private static String toString(@Nullable StackTraceElement[] stackTrace) {
+ if (stackTrace != null) {
+ StringBuilder threadDetails = new StringBuilder();
+ Stream.of(stackTrace).forEach(threadDetails::append);
+ return threadDetails.toString();
+ } else {
+ return "N/A";
+ }
+ }
+
+ @NotNull
+ private static String toString(@Nullable GCGeneration gcGeneration) {
+ return gcGeneration == null
+ ? "N/A"
+ : gcGeneration.toString();
}
@Override
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java?rev=1855590&r1=1855589&r2=1855590&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/scheduler/LockBasedScheduler.java Fri Mar 15 13:35:08 2019
@@ -257,7 +257,7 @@ public class LockBasedScheduler implemen
commitSemaphoreLogging.warnOnBlockingCommit();
long queuedTime = System.nanoTime();
- stats.onCommitQueued(Thread.currentThread());
+ stats.onCommitQueued(Thread.currentThread(), commit::getGCGeneration);
commitSemaphore.acquire();
commitSemaphoreLogging.commitStarted(commit);
Modified: jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CommitsTrackerTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CommitsTrackerTest.java?rev=1855590&r1=1855589&r2=1855590&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CommitsTrackerTest.java (original)
+++ jackrabbit/oak/trunk/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/CommitsTrackerTest.java Fri Mar 15 13:35:08 2019
@@ -21,13 +21,17 @@ package org.apache.jackrabbit.oak.segmen
import static com.google.common.collect.Lists.newArrayList;
import static java.lang.Math.min;
+import static org.apache.jackrabbit.oak.segment.file.tar.GCGeneration.newGCGeneration;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.util.List;
import java.util.Map;
+import org.apache.jackrabbit.oak.segment.CommitsTracker.Commit;
+import org.apache.jackrabbit.oak.segment.file.tar.GCGeneration;
import org.junit.Test;
public class CommitsTrackerTest {
@@ -35,19 +39,22 @@ public class CommitsTrackerTest {
private static class CommitTask {
private final CommitsTracker commitsTracker;
private final Thread thread;
+ private final GCGeneration gcGeneration;
- CommitTask(CommitsTracker commitsTracker) {
+ CommitTask(CommitsTracker commitsTracker, GCGeneration gcGeneration) {
this.commitsTracker = commitsTracker;
this.thread = new Thread();
+ this.gcGeneration = gcGeneration;
}
- CommitTask(CommitsTracker commitsTracker, String threadName) {
+ CommitTask(CommitsTracker commitsTracker, String threadName, GCGeneration gcGeneration) {
this.commitsTracker = commitsTracker;
this.thread = new Thread(threadName);
+ this.gcGeneration = gcGeneration;
}
public void queued() {
- commitsTracker.trackQueuedCommitOf(thread);
+ commitsTracker.trackQueuedCommitOf(thread, () -> gcGeneration);
}
public void dequeue() {
@@ -61,31 +68,42 @@ public class CommitsTrackerTest {
public String getThreadName() {
return thread.getName();
}
+
+ public GCGeneration getGcGeneration() {
+ return gcGeneration;
+ }
}
@Test
public void testCommitsCountOthers() throws InterruptedException {
- CommitsTracker commitsTracker = new CommitsTracker(new String[] {}, 10);
+ final int OTHER_WRITERS_LIMIT = 10;
+ CommitsTracker commitsTracker = new CommitsTracker(new String[] {}, OTHER_WRITERS_LIMIT);
List<CommitTask> queued = newArrayList();
for (int k = 0; k < 20; k++) {
- CommitTask commitTask = new CommitTask(commitsTracker);
+ CommitTask commitTask = new CommitTask(commitsTracker, newGCGeneration(k, k, false));
queued.add(commitTask);
commitTask.queued();
+ assertNull(commitsTracker.getCurrentWriter());
assertEquals(queued.size(), commitsTracker.getQueuedWritersMap().size());
assertEquals(0, commitsTracker.getCommitsCountOthers().size());
assertTrue(commitsTracker.getCommitsCountPerGroupLastMinute().isEmpty());
}
List<CommitTask> executed = newArrayList();
- for (int k = 0; k < 13; k ++) {
+ for (int k = 0; k < OTHER_WRITERS_LIMIT + 3; k++) {
CommitTask commitTask = queued.remove(0);
executed.add(commitTask);
commitTask.dequeue();
+ Commit currentWriter = commitsTracker.getCurrentWriter();
+ assertNotNull(currentWriter);
+ assertEquals(commitTask.getGcGeneration(), currentWriter.getGCGeneration());
+ assertEquals(commitTask.getThreadName(), commitsTracker.getCurrentWriter().getThreadName());
commitTask.executed();
+ assertNull(commitsTracker.getCurrentWriter());
assertEquals(queued.size(), commitsTracker.getQueuedWritersMap().size());
- assertEquals(min(10, executed.size()), commitsTracker.getCommitsCountOthers().size());
+ assertEquals(min(OTHER_WRITERS_LIMIT, executed.size()), commitsTracker.getCommitsCountOthers().size());
assertTrue(commitsTracker.getCommitsCountPerGroupLastMinute().isEmpty());
}
}
@@ -96,7 +114,10 @@ public class CommitsTrackerTest {
CommitsTracker commitsTracker = new CommitsTracker(groups, 10);
for (int k = 0; k < 40; k++) {
- CommitTask commitTask = new CommitTask(commitsTracker, "Thread-" + (10 + k));
+ CommitTask commitTask = new CommitTask(
+ commitsTracker,
+ "Thread-" + (10 + k),
+ GCGeneration.NULL);
commitTask.queued();
commitTask.dequeue();
commitTask.executed();