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 ca...@apache.org on 2018/02/28 01:38:09 UTC
svn commit: r1825523 - in /jackrabbit/oak/trunk/oak-run/src:
main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/
test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/
Author: catholicon
Date: Wed Feb 28 01:38:09 2018
New Revision: 1825523
URL: http://svn.apache.org/viewvc?rev=1825523&view=rev
Log:
OAK-7284: Reindexing using --doc-traversal-mode can hit ConcurrentModificationException during aggregation
CursorableLinkedList led to heavy drop in performance as we weren't closing cursor.
And without that, the class would broadcast all changes to all open cursors.
Adding lifecycle mgmt was a mess, so cooking up our own poor man link list for our own purpose.
Btw, the list impl is not generics - because YAGNI
Added:
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java (with props)
jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java (with props)
Modified:
jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java
Added: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java?rev=1825523&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java (added)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java Wed Feb 28 01:38:09 2018
@@ -0,0 +1,124 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.index.indexer.document.flatfile;
+
+import com.google.common.base.Preconditions;
+import org.apache.jackrabbit.oak.index.indexer.document.NodeStateEntry;
+
+import javax.annotation.Nonnull;
+import java.util.Iterator;
+
+import static org.apache.jackrabbit.oak.index.indexer.document.flatfile.FlatFileBufferLinkedList.NodeIterator.iteratorFor;
+
+/**
+ * Linked list implementation which supports multiple iterators. The iterator's state
+ * is backed by an actual node in the list. So, modification in the list show up in
+ * iterator (assuming thread safely/volatility) getting handled outside of the class.
+ */
+public class FlatFileBufferLinkedList {
+
+ private ListNode head = new ListNode();
+ private ListNode tail = head;
+
+ private int size = 0;
+
+ /**
+ * Add {@code item} at the tail of the list
+ */
+ public void add(@Nonnull NodeStateEntry item) {
+ tail.next = new ListNode(item);
+ tail = tail.next;
+ size++;
+ }
+
+ /**
+ * Remove the first item from the list
+ * @return {@code NodeStateEntry} data in the removed item
+ */
+ public NodeStateEntry remove() {
+ Preconditions.checkState(!isEmpty(), "Cannot remove item from empty list");
+ NodeStateEntry ret = head.next.data;
+ head.next.isValid = false;
+ head.next = head.next.next;
+ size--;
+ if (size == 0) {
+ tail = head;
+ }
+ return ret;
+ }
+
+ /**
+ * @return {@link NodeIterator} object which would iterate the whole list
+ */
+ public NodeIterator iterator() {
+ return iteratorFor(head);
+ }
+
+ public int size() {
+ return size;
+ }
+
+ public boolean isEmpty() {
+ return size == 0;
+ }
+
+ /**
+ * Represents an item in the list.
+ */
+ static class ListNode {
+ private ListNode next;
+ private final NodeStateEntry data;
+ private boolean isValid = true;
+
+ private ListNode() {
+ this.data = null;
+ this.next = null;
+ }
+
+ ListNode(@Nonnull NodeStateEntry data) {
+ this.data = data;
+ this.next = null;
+ }
+ }
+
+ static class NodeIterator implements Iterator<NodeStateEntry> {
+ private ListNode current;
+
+ static NodeIterator iteratorFor(@Nonnull ListNode node) {
+ return new NodeIterator(node);
+ }
+
+ NodeIterator(@Nonnull ListNode start) {
+ this.current = start;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return current.next != null;
+ }
+
+ @Override
+ public NodeStateEntry next() {
+ Preconditions.checkState(current.isValid, "Can't call next from a removed node");
+ current = current.next;
+ return current.data;
+ }
+ }
+}
Propchange: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedList.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified: jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java?rev=1825523&r1=1825522&r2=1825523&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java (original)
+++ jackrabbit/oak/trunk/oak-run/src/main/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileStoreIterator.java Wed Feb 28 01:38:09 2018
@@ -20,12 +20,11 @@
package org.apache.jackrabbit.oak.index.indexer.document.flatfile;
import java.util.Iterator;
-import java.util.ListIterator;
import java.util.Set;
import com.google.common.collect.AbstractIterator;
-import org.apache.commons.collections.list.CursorableLinkedList;
import org.apache.jackrabbit.oak.index.indexer.document.NodeStateEntry;
+import org.apache.jackrabbit.oak.index.indexer.document.flatfile.FlatFileBufferLinkedList.NodeIterator;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -36,7 +35,7 @@ import static com.google.common.collect.
class FlatFileStoreIterator extends AbstractIterator<NodeStateEntry> implements Iterator<NodeStateEntry> {
private final Logger log = LoggerFactory.getLogger(getClass());
private final Iterator<NodeStateEntry> baseItr;
- private final CursorableLinkedList buffer = new CursorableLinkedList();
+ private final FlatFileBufferLinkedList buffer = new FlatFileBufferLinkedList();
private NodeStateEntry current;
private final Set<String> preferredPathElements;
private int maxBufferSize;
@@ -68,7 +67,7 @@ class FlatFileStoreIterator extends Abst
log.info("Max buffer size changed {} for path {}", maxBufferSize, current.getPath());
}
if (!buffer.isEmpty()) {
- return (NodeStateEntry)buffer.removeFirst();
+ return buffer.remove();
}
if (baseItr.hasNext()) {
return wrap(baseItr.next());
@@ -87,14 +86,13 @@ class FlatFileStoreIterator extends Abst
}
private Iterator<NodeStateEntry> queueIterator() {
- ListIterator<NodeStateEntry> qitr = buffer.listIterator();
+ NodeIterator qitr = buffer.iterator();
return new AbstractIterator<NodeStateEntry>() {
@Override
protected NodeStateEntry computeNext() {
//If queue is empty try to append by getting entry from base
if (!qitr.hasNext() && baseItr.hasNext()) {
- qitr.add(wrap(baseItr.next()));
- qitr.previous(); //Move back the itr
+ buffer.add(wrap(baseItr.next()));
}
if (qitr.hasNext()) {
return qitr.next();
Added: jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java?rev=1825523&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java (added)
+++ jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java Wed Feb 28 01:38:09 2018
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.jackrabbit.oak.index.indexer.document.flatfile;
+
+import com.google.common.collect.Iterators;
+import org.apache.jackrabbit.oak.index.indexer.document.NodeStateEntry;
+import org.apache.jackrabbit.oak.index.indexer.document.flatfile.FlatFileBufferLinkedList.NodeIterator;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class FlatFileBufferLinkedListTest {
+ private FlatFileBufferLinkedList list = null;
+ private static final NodeStateEntry TEST_NODE_STATE_ENTRY = new NodeStateEntry(EMPTY_NODE, "/");
+
+ @Before
+ public void setup() {
+ list = new FlatFileBufferLinkedList();
+ }
+
+ @Test
+ public void add() {
+ try {
+ list.add(null);
+ fail("Adding null must throw IllegalArgumentException");
+ } catch (IllegalArgumentException iae) {
+ //ignore
+ }
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ }
+
+ @Test
+ public void remove() {
+ try {
+ list.remove();
+ fail("Must fail to remove from an empty list");
+ } catch (IllegalStateException ise) {
+ //ignore
+ }
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("Should get item on removal", TEST_NODE_STATE_ENTRY, list.remove());
+ }
+
+ @Test
+ public void iterator() {
+ assertEquals("empty list must be 0-sized", 0, Iterators.size(list.iterator()));
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("single entry list must be 1-sized", 1, Iterators.size(list.iterator()));
+ assertEquals("single entry list must be 1-sized on separate iterators",
+ 1, Iterators.size(list.iterator()));
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("2 entries in list must be 2-sized", 2, Iterators.size(list.iterator()));
+
+ assertEquals("2 entries in list must be 2-sized on separate iterators",
+ 2, Iterators.size(list.iterator()));
+
+ NodeIterator iter2 = list.iterator();
+ NodeIterator iter1 = list.iterator();
+ iter2.next();
+ assertEquals("2 entries in list must be 1-sized after consuming an item",
+ 1, Iterators.size(iter2));
+ assertEquals("2 entries in list must be 2-sized even if some other iterator consumed an item",
+ 2, Iterators.size(iter1));
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ iter1 = list.iterator();
+ iter2 = list.iterator();
+
+ iter1.next();//move iter to point at node being removed below
+ iter2.next();iter2.next();// move iter beyond node being removed - this should remain valid
+
+ list.remove();
+ try {
+ iter1.next();
+ fail("Iterator state once removed from list can't be traversed");
+ } catch (IllegalStateException ise) {
+ //ignore
+ }
+ assertEquals(TEST_NODE_STATE_ENTRY, iter2.next());//this should work
+ assertEquals("2 entries in list must be 1-sized after removal of an iterm",
+ 2, Iterators.size(list.iterator()));
+ }
+
+ @Test
+ public void size() {
+ assertEquals("empty list must be 0-sized", 0, list.size());
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("single entry list must be 1-sized", 1, list.size());
+ assertEquals("single entry list must be 1-sized on separate iterators", 1, list.size());
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("2 entries in list must be 2-sized", 2, list.size());
+
+ assertEquals("2 entries in list must be 2-sized on separate iterators", 2, list.size());
+
+ list.remove();
+ assertEquals("2 entries in list must be 1-sized after removing an item", 1, list.size());
+ }
+
+ @Test
+ public void isEmpty() {
+ assertTrue("Empty list should be empty", list.isEmpty());
+
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertFalse("Non-empty list should be non-empty", list.isEmpty());
+
+ list.remove();
+ assertTrue("Empty list due to removal should be empty", list.isEmpty());
+ }
+
+ @Test
+ public void basics() {
+ list.add(TEST_NODE_STATE_ENTRY);
+ assertEquals("Adding an item should change size", 1, list.size());
+ assertTrue("Adding an item should be available", list.iterator().hasNext());
+ }
+}
\ No newline at end of file
Propchange: jackrabbit/oak/trunk/oak-run/src/test/java/org/apache/jackrabbit/oak/index/indexer/document/flatfile/FlatFileBufferLinkedListTest.java
------------------------------------------------------------------------------
svn:eol-style = native