Author: adulceanu
Date: Thu Jun 4 08:52:33 2020
New Revision: 1878468
URL: http://svn.apache.org/viewvc?rev=1878468&view=rev
Log:
OAK-9095 - MapRecord corruption when adding more than MapRecord.MAX_SIZE
entries in branch record (backported r1878464 to 1.22 branch)
Modified:
jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/segment/records.md
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
jackrabbit/oak/branches/1.22/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java
Modified:
jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/segment/records.md
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/segment/records.md?rev=1878468&r1=1878467&r2=1878468&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/segment/records.md
(original)
+++
jackrabbit/oak/branches/1.22/oak-doc/src/site/markdown/nodestore/segment/records.md
Thu Jun 4 08:52:33 2020
@@ -292,6 +292,8 @@ only a "diff" of the map is stored. This
modified map, which can save a considerable amount of space if the original map
was big.
+_Warning: A map record can store up to 2^29 - 1 (i.e. 536.870.911) entries! In
order to avoid reaching this number and possibly running into issues from
surpassing it, log messages are printed after reaching 400.000.000 entries and
writing beyond 500.000.000 entries is not allowed unless the boolean system
property `oak.segmentNodeStore.allowWritesOnHugeMapRecord` is set. Finally, the
segment store does not allow writing map records with more than 536.000.000
entries._
+
### Template records
A template record stores metadata about nodes that, on average, don't change so
Modified:
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java?rev=1878468&r1=1878467&r2=1878468&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java
(original)
+++
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriter.java
Thu Jun 4 08:52:33 2020
@@ -211,6 +211,8 @@ public class DefaultSegmentWriter implem
private final Cache<String, RecordId> nodeCache;
+ private long lastLogTime;
+
SegmentWriteOperation(@NotNull GCGeneration gcGeneration) {
int generation = gcGeneration.getGeneration();
this.gcGeneration = gcGeneration;
@@ -223,10 +225,46 @@ public class DefaultSegmentWriter implem
return writer -> recordWriter.write(writer, store);
}
- private RecordId writeMap(@Nullable MapRecord base,
- @NotNull Map<String, RecordId> changes
- )
- throws IOException {
+ private boolean shouldLog() {
+ long now = System.currentTimeMillis();
+ if (now - lastLogTime > 1000) {
+ lastLogTime = now;
+ return true;
+ } else {
+ return false;
+ }
+ }
+
+ private RecordId writeMap(@Nullable MapRecord base, @NotNull
Map<String, RecordId> changes) throws IOException {
+ if (base != null) {
+ if (base.size() >= MapRecord.WARN_SIZE) {
+ if (base.size() >= MapRecord.ERROR_SIZE_HARD_STOP) {
+ throw new UnsupportedOperationException("Map record
has more than " + MapRecord.ERROR_SIZE_HARD_STOP
+ + " direct entries. Writing is not
allowed. Please remove entries.");
+ } else if (base.size() >=
MapRecord.ERROR_SIZE_DISCARD_WRITES) {
+ if
(!Boolean.getBoolean("oak.segmentNodeStore.allowWritesOnHugeMapRecord")) {
+ if (shouldLog()) {
+ LOG.error(
+ "Map entry has more than {} entries.
Writing more than {} entries (up to the hard limit of {}) is only allowed "
+ + "if the system property
\"oak.segmentNodeStore.allowWritesOnHugeMapRecord\" is set",
+ MapRecord.ERROR_SIZE,
MapRecord.ERROR_SIZE_DISCARD_WRITES, MapRecord.ERROR_SIZE_HARD_STOP);
+ }
+
+ throw new UnsupportedOperationException("Map
record has more than " + MapRecord.ERROR_SIZE_DISCARD_WRITES
+ + " direct entries. Writing is not
allowed. Please remove entries.");
+ }
+ } else if (base.size() >= MapRecord.ERROR_SIZE) {
+ if (shouldLog()) {
+ LOG.error("Map entry has more than {} entries.
Please remove entries.", MapRecord.ERROR_SIZE);
+ }
+ } else {
+ if (shouldLog()) {
+ LOG.warn("Map entry has more than {} entries.
Please remove entries.", MapRecord.WARN_SIZE);
+ }
+ }
+ }
+ }
+
if (base != null && base.isDiff()) {
Segment segment = base.getSegment();
RecordId key = segment.readRecordId(base.getRecordNumber(), 8);
@@ -289,6 +327,7 @@ public class DefaultSegmentWriter implem
}
private RecordId writeMapBranch(int level, int size, MapRecord...
buckets) throws IOException {
+ checkElementIndex(size, MapRecord.MAX_SIZE);
int bitmap = 0;
List<RecordId> bucketIds =
newArrayListWithCapacity(buckets.length);
for (int i = 0; i < buckets.length; i++) {
Modified:
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java?rev=1878468&r1=1878467&r2=1878468&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
(original)
+++
jackrabbit/oak/branches/1.22/oak-segment-tar/src/main/java/org/apache/jackrabbit/oak/segment/MapRecord.java
Thu Jun 4 08:52:33 2020
@@ -97,6 +97,28 @@ public class MapRecord extends Record {
*/
protected static final int MAX_SIZE = (1 << SIZE_BITS) - 1; // ~268e6
+ /**
+ * Going over this limit will generate a warning message in the log
+ */
+ protected static final int WARN_SIZE = 400_000_000;
+
+ /**
+ * Going over this limit will generate an error message in the log
+ */
+ protected static final int ERROR_SIZE = 450_000_000;
+
+ /**
+ * Going over this limit will generate an error message in the log and
won't allow writes
+ * unless <code>oak.segmentNodeStore.allowWritesOnHugeMapRecord</code>
system property is set.
+ */
+ protected static final int ERROR_SIZE_DISCARD_WRITES = 500_000_000;
+
+ /**
+ * Going over this limit will generate an error message in the log and
won't allow any writes
+ * whatsoever. Moreover {@link UnsupportedOperationException} will be
thrown.
+ */
+ protected static final int ERROR_SIZE_HARD_STOP = 536_000_000;
+
MapRecord(@NotNull SegmentReader reader, @NotNull RecordId id) {
super(id);
this.reader = checkNotNull(reader);
@@ -180,7 +202,7 @@ public class MapRecord extends Record {
int level = getLevel(head);
if (isBranch(size, level)) {
// this is an intermediate branch record
- // check if a matching bucket exists, and recurse
+ // check if a matching bucket exists, and recurse
int bitmap = segment.readInt(getRecordNumber(), 4);
int mask = (1 << BITS_PER_LEVEL) - 1;
int shift = 32 - (level + 1) * BITS_PER_LEVEL;
Modified:
jackrabbit/oak/branches/1.22/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.22/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java?rev=1878468&r1=1878467&r2=1878468&view=diff
==============================================================================
---
jackrabbit/oak/branches/1.22/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java
(original)
+++
jackrabbit/oak/branches/1.22/oak-segment-tar/src/test/java/org/apache/jackrabbit/oak/segment/DefaultSegmentWriterTest.java
Thu Jun 4 08:52:33 2020
@@ -29,23 +29,33 @@ import static org.junit.Assert.assertFal
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
-import java.io.File;
-import java.io.IOException;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-
import com.google.common.base.Charsets;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableMap;
+
import org.apache.jackrabbit.oak.segment.test.TemporaryFileStore;
+import org.jetbrains.annotations.NotNull;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.RuleChain;
import org.junit.rules.TemporaryFolder;
+import org.mockito.Mockito;
+import org.slf4j.LoggerFactory;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.Appender;
+import ch.qos.logback.core.read.ListAppender;
public class DefaultSegmentWriterTest {
@@ -234,6 +244,71 @@ public class DefaultSegmentWriterTest {
assertNull(many.getEntry("foo"));
}
+ @Test(expected = UnsupportedOperationException.class)
+ public void testHugeMapRecordErrorSizeDiscardWrites() throws IOException {
+ RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
+
+ MapRecord one = new MapRecord(store.fileStore().getReader(),
writer.writeMap(null, ImmutableMap.of("one", blockId)));
+ MapRecord hugeMapRecord = Mockito.spy(one);
+
Mockito.when(hugeMapRecord.size()).thenReturn(MapRecord.ERROR_SIZE_DISCARD_WRITES);
+
+ MapRecord many = new MapRecord(store.fileStore().getReader(),
writer.writeMap(hugeMapRecord, ImmutableMap.of("one", blockId)));
+ }
+
+ @Test
+ public void testHugeMapRecordAllowdWritesWithSystemProperty() throws
IOException {
+ System.setProperty("oak.segmentNodeStore.allowWritesOnHugeMapRecord",
"true");
+ RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
+
+ MapRecord one = new MapRecord(store.fileStore().getReader(),
writer.writeMap(null, ImmutableMap.of("one", blockId)));
+ MapRecord hugeMapRecord = Mockito.spy(one);
+
Mockito.when(hugeMapRecord.size()).thenReturn(MapRecord.ERROR_SIZE_DISCARD_WRITES);
+
+ MapRecord many = new MapRecord(store.fileStore().getReader(),
writer.writeMap(hugeMapRecord, ImmutableMap.of("one", blockId)));
+ }
+
+ @Test(expected = UnsupportedOperationException.class)
+ public void testHugeMapRecordErrorSizeHardStop() throws IOException {
+ System.setProperty("oak.segmentNodeStore.allowWritesOnHugeMapRecord",
"true");
+ RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
+
+ MapRecord one = new MapRecord(store.fileStore().getReader(),
writer.writeMap(null, ImmutableMap.of("one", blockId)));
+ MapRecord hugeMapRecord = Mockito.spy(one);
+
Mockito.when(hugeMapRecord.size()).thenReturn(MapRecord.ERROR_SIZE_HARD_STOP);
+
+ MapRecord many = new MapRecord(store.fileStore().getReader(),
writer.writeMap(hugeMapRecord, ImmutableMap.of("one", blockId)));
+ }
+
+ @Test
+ public void testHugeMapRecordErrorSize() throws IOException {
+ RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
+ final ListAppender<ILoggingEvent> logAppender = subscribeAppender();
+
+ MapRecord one = new MapRecord(store.fileStore().getReader(),
writer.writeMap(null, ImmutableMap.of("one", blockId)));
+ MapRecord hugeMapRecord = Mockito.spy(one);
+ Mockito.when(hugeMapRecord.size()).thenReturn(MapRecord.ERROR_SIZE);
+
+ MapRecord many = new MapRecord(store.fileStore().getReader(),
writer.writeMap(hugeMapRecord, ImmutableMap.of("one", blockId)));
+ assertEquals(logAppender.list.get(0).getFormattedMessage(), "Map entry
has more than 450000000 entries. Please remove entries.");
+ assertEquals(logAppender.list.get(0).getLevel(), Level.ERROR);
+ unsubscribe(logAppender);
+ }
+
+ @Test
+ public void testHugeMapRecordWarnSize() throws IOException {
+ RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
+ final ListAppender<ILoggingEvent> logAppender = subscribeAppender();
+
+ MapRecord one = new MapRecord(store.fileStore().getReader(),
writer.writeMap(null, ImmutableMap.of("one", blockId)));
+ MapRecord hugeMapRecord = Mockito.spy(one);
+ Mockito.when(hugeMapRecord.size()).thenReturn(MapRecord.WARN_SIZE);
+
+ MapRecord many = new MapRecord(store.fileStore().getReader(),
writer.writeMap(hugeMapRecord, ImmutableMap.of("one", blockId)));
+ assertEquals(logAppender.list.get(0).getFormattedMessage(), "Map entry
has more than 400000000 entries. Please remove entries.");
+ assertEquals(logAppender.list.get(0).getLevel(), Level.WARN);
+ unsubscribe(logAppender);
+ }
+
@Test
public void testMapRemoveNonExisting() throws IOException {
RecordId blockId = writer.writeBlock(bytes, 0, bytes.length);
@@ -294,4 +369,19 @@ public class DefaultSegmentWriterTest {
}
}
+ private ListAppender<ILoggingEvent> subscribeAppender() {
+ ListAppender<ILoggingEvent> appender = new
ListAppender<ILoggingEvent>();
+ appender.setContext((LoggerContext) LoggerFactory.getILoggerFactory());
+ appender.setName("asynclogcollector");
+ appender.start();
+ ((LoggerContext) LoggerFactory.getILoggerFactory()).getLogger(
+
ch.qos.logback.classic.Logger.ROOT_LOGGER_NAME).addAppender(appender);
+ return appender;
+
+ }
+
+ private void unsubscribe(@NotNull final Appender<ILoggingEvent> appender) {
+ ((LoggerContext) LoggerFactory.getILoggerFactory()).getLogger(
+
ch.qos.logback.classic.Logger.ROOT_LOGGER_NAME).detachAppender(appender);
+ }
}
