Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java (revision 1660375) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Segment.java (working copy) @@ -54,10 +54,11 @@ /** * Version of the segment storage format. * */ - public static final byte STORAGE_FORMAT_VERSION = 10; + public static final byte STORAGE_FORMAT_VERSION = 11; /** * Number of bytes used for storing a record identifier. One byte @@ -116,6 +117,8 @@ private final ByteBuffer data; + private final byte segmentFormat; + /** * Referenced segment identifiers. Entries are initialized lazily in * {@link #getRefId(int)}. Set to {@code null} for bulk segments. @@ -142,13 +145,16 @@ this.data = checkNotNull(data); if (id.isDataSegmentId()) { + segmentFormat = data.get(3); checkState(data.get(0) == '0' && data.get(1) == 'a' && data.get(2) == 'K' - && data.get(3) == STORAGE_FORMAT_VERSION); + && (segmentFormat == STORAGE_FORMAT_VERSION - 1 // backward compatible with previous format + || segmentFormat == STORAGE_FORMAT_VERSION)); this.refids = new SegmentId[getRefCount()]; refids[0] = id; } else { + this.segmentFormat = STORAGE_FORMAT_VERSION; this.refids = null; } } @@ -159,6 +165,7 @@ this.data = ByteBuffer.wrap(checkNotNull(buffer)); this.refids = new SegmentId[SEGMENT_REFERENCE_LIMIT + 1]; + this.segmentFormat = STORAGE_FORMAT_VERSION; refids[0] = id; } @@ -171,6 +178,10 @@ return accessed != 0; } + byte getStorageFormatVersion() { + return segmentFormat; + } + /** * Maps the given record offset to the respective position within the * internal {@link #data} array. The validity of a record with the given @@ -436,19 +447,41 @@ offset += Segment.RECORD_ID_BYTES; } - PropertyTemplate[] properties = - new PropertyTemplate[propertyCount]; - for (int i = 0; i < properties.length; i++) { + PropertyTemplate[] properties; + if (segmentFormat == STORAGE_FORMAT_VERSION) { + properties = readPropsV11(propertyCount, offset); + } else { + properties = readPropsV10(propertyCount, offset); + } + return new Template(primaryType, mixinTypes, properties, childName); + } + + private PropertyTemplate[] readPropsV10(int propertyCount, int offset) { + PropertyTemplate[] properties = new PropertyTemplate[propertyCount]; + for (int i = 0; i < propertyCount; i++) { RecordId propertyNameId = readRecordId(offset); offset += Segment.RECORD_ID_BYTES; byte type = readByte(offset++); - properties[i] = new PropertyTemplate( - i, readString(propertyNameId), + properties[i] = new PropertyTemplate(i, readString(propertyNameId), Type.fromTag(Math.abs(type), type < 0)); } + return properties; + } - return new Template( - primaryType, mixinTypes, properties, childName); + private PropertyTemplate[] readPropsV11(int propertyCount, int offset) { + PropertyTemplate[] properties = new PropertyTemplate[propertyCount]; + if (propertyCount > 0) { + RecordId id = readRecordId(offset); + ListRecord propertyNames = new ListRecord(id, properties.length); + offset += Segment.RECORD_ID_BYTES; + for (int i = 0; i < propertyCount; i++) { + byte type = readByte(offset++); + properties[i] = new PropertyTemplate(i, + readString(propertyNames.getEntry(i)), Type.fromTag( + Math.abs(type), type < 0)); + } + } + return properties; } long readLength(RecordId id) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java (revision 1660375) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentNodeState.java (working copy) @@ -134,17 +134,39 @@ template.getPropertyTemplate(name); if (propertyTemplate != null) { Segment segment = getSegment(); - int ids = 1 + propertyTemplate.getIndex(); - if (template.getChildName() != Template.ZERO_CHILD_NODES) { - ids++; + RecordId id; + if (getSegment().getStorageFormatVersion() == Segment.STORAGE_FORMAT_VERSION) { + id = getRecordIdV11(segment, template, propertyTemplate); + } else { + id = getRecordIdV10(segment, template, propertyTemplate); } - return new SegmentPropertyState( - segment.readRecordId(getOffset(0, ids)), propertyTemplate); + return new SegmentPropertyState(id, propertyTemplate); } else { return null; } } + private RecordId getRecordIdV10(Segment segment, Template template, + PropertyTemplate propertyTemplate) { + int ids = 1 + propertyTemplate.getIndex(); + if (template.getChildName() != Template.ZERO_CHILD_NODES) { + ids++; + } + return segment.readRecordId(getOffset(0, ids)); + } + + private RecordId getRecordIdV11(Segment segment, Template template, + PropertyTemplate propertyTemplate) { + int ids = 1; + if (template.getChildName() != Template.ZERO_CHILD_NODES) { + ids++; + } + RecordId rid = segment.readRecordId(getOffset(0, ids)); + ListRecord pIds = new ListRecord(rid, + template.getPropertyTemplates().length); + return pIds.getEntry(propertyTemplate.getIndex()); + } + @Override @Nonnull public Iterable getProperties() { Template template = getTemplate(); @@ -167,10 +189,26 @@ if (template.getChildName() != Template.ZERO_CHILD_NODES) { ids++; } - for (int i = 0; i < propertyTemplates.length; i++) { - RecordId propertyId = segment.readRecordId(getOffset(0, ids++)); - list.add(new SegmentPropertyState( - propertyId, propertyTemplates[i])); + + if (segment.getStorageFormatVersion() == Segment.STORAGE_FORMAT_VERSION) { + // -- V11 -- + if (propertyTemplates.length > 0) { + ListRecord pIds = new ListRecord( + segment.readRecordId(getOffset(0, ids)), + propertyTemplates.length); + for (int i = 0; i < propertyTemplates.length; i++) { + RecordId propertyId = pIds.getEntry(i); + list.add(new SegmentPropertyState(propertyId, + propertyTemplates[i])); + } + } + } else { + // -- V10 -- + for (int i = 0; i < propertyTemplates.length; i++) { + RecordId propertyId = segment.readRecordId(getOffset(0, ids++)); + list.add(new SegmentPropertyState(propertyId, + propertyTemplates[i])); + } } return list; @@ -247,11 +285,13 @@ } Segment segment = getSegment(); - int ids = 1 + propertyTemplate.getIndex(); - if (template.getChildName() != Template.ZERO_CHILD_NODES) { - ids++; + RecordId id; + if (getSegment().getStorageFormatVersion() == Segment.STORAGE_FORMAT_VERSION) { + id = getRecordIdV11(segment, template, propertyTemplate); + } else { + id = getRecordIdV10(segment, template, propertyTemplate); } - return segment.readString(segment.readRecordId(getOffset(0, ids))); + return segment.readString(id); } /** @@ -288,12 +328,12 @@ } Segment segment = getSegment(); - int ids = 1 + propertyTemplate.getIndex(); - if (template.getChildName() != Template.ZERO_CHILD_NODES) { - ids++; + RecordId id; + if (getSegment().getStorageFormatVersion() == Segment.STORAGE_FORMAT_VERSION) { + id = getRecordIdV11(segment, template, propertyTemplate); + } else { + id = getRecordIdV10(segment, template, propertyTemplate); } - - RecordId id = segment.readRecordId(getOffset(0, ids)); segment = id.getSegment(); int size = segment.readInt(id.getOffset()); if (size == 0) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java (revision 1660375) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/SegmentWriter.java (working copy) @@ -119,6 +119,7 @@ * avoid storing duplicates of frequently occurring data. * Should only be accessed from synchronized blocks to prevent corruption. */ + @SuppressWarnings("serial") private final Map records = new LinkedHashMap(15000, 0.75f, true) { @Override @@ -444,7 +445,6 @@ } } - private synchronized RecordId writeListBucket(List bucket) { checkArgument(bucket.size() > 1); RecordId bucketId = prepare(RecordType.BUCKET, 0, bucket); @@ -968,7 +968,13 @@ propertyTypes[i] = (byte) type.tag(); } } - ids.addAll(Arrays.asList(propertyNames)); + + RecordId propNamesId = null; + if (propertyNames.length > 0) { + propNamesId = writeList(Arrays.asList(propertyNames)); + ids.add(propNamesId); + } + checkState(propertyNames.length < (1 << 18)); head |= propertyNames.length; @@ -985,8 +991,10 @@ if (childNameId != null) { writeRecordId(childNameId); } + if (propNamesId != null) { + writeRecordId(propNamesId); + } for (int i = 0; i < propertyNames.length; i++) { - writeRecordId(propertyNames[i]); buffer[position++] = propertyTypes[i]; } @@ -1087,36 +1095,42 @@ ids.add(writeNode(state.getChildNode(template.getChildName())).getRecordId()); } + List pIds = Lists.newArrayList(); for (PropertyTemplate pt : template.getPropertyTemplates()) { String name = pt.getName(); PropertyState property = state.getProperty(name); if (property instanceof SegmentPropertyState && store.containsSegment(((SegmentPropertyState) property).getRecordId().getSegmentId())) { - ids.add(((SegmentPropertyState) property).getRecordId()); + pIds.add(((SegmentPropertyState) property).getRecordId()); } else if (before == null || !store.containsSegment(before.getRecordId().getSegmentId())) { - ids.add(writeProperty(property)); + pIds.add(writeProperty(property)); } else { // reuse previously stored property, if possible PropertyTemplate bt = beforeTemplate.getPropertyTemplate(name); if (bt == null) { - ids.add(writeProperty(property)); // new property + RecordId p = writeProperty(property); + pIds.add(p); // new property } else { SegmentPropertyState bp = beforeTemplate.getProperty( before.getRecordId(), bt.getIndex()); if (property.equals(bp)) { - ids.add(bp.getRecordId()); // no changes + pIds.add(bp.getRecordId()); // no changes } else if (bp.isArray() && bp.getType() != BINARIES) { // reuse entries from the previous list - ids.add(writeProperty(property, bp.getValueRecords())); + pIds.add(writeProperty(property, bp.getValueRecords())); } else { - ids.add(writeProperty(property)); + pIds.add(writeProperty(property)); } } } } + if (!pIds.isEmpty()) { + ids.add(writeList(pIds)); + } + synchronized (this) { RecordId recordId = prepare(RecordType.NODE, 0, ids); for (RecordId id : ids) { Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java (revision 1660375) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/Template.java (working copy) @@ -173,9 +173,19 @@ if (childName != ZERO_CHILD_NODES) { offset += RECORD_ID_BYTES; } - offset += index * RECORD_ID_BYTES; - return new SegmentPropertyState( - segment.readRecordId(offset), properties[index]); + + RecordId rid = null; + if (segment.getStorageFormatVersion() == Segment.STORAGE_FORMAT_VERSION) { + // read storage format as V11 + RecordId lid = segment.readRecordId(offset); + ListRecord props = new ListRecord(lid, properties.length); + rid = props.getEntry(index); + } else { + // read storage format as V10 + offset += index * RECORD_ID_BYTES; + rid = segment.readRecordId(offset); + } + return new SegmentPropertyState(rid, properties[index]); } MapRecord getChildNodeMap(RecordId recordId) { Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentSizeTest.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentSizeTest.java (revision 1660375) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/SegmentSizeTest.java (working copy) @@ -51,8 +51,8 @@ builder = EMPTY_NODE.builder(); builder.setProperty("foo", "bar"); builder.setProperty("baz", 123); - assertEquals(64, getSize(builder)); - assertEquals(12, getAmortizedSize(builder)); + assertEquals(80, getSize(builder)); + assertEquals(16, getAmortizedSize(builder)); builder = EMPTY_NODE.builder(); builder.child("foo"); @@ -127,15 +127,15 @@ deny.setProperty(PropertyStates.createProperty( "rep:privileges", ImmutableList.of("jcr:read"), Type.NAMES)); assertEquals(176, getSize(builder)); - assertEquals(28, getAmortizedSize(builder)); + assertEquals(32, getAmortizedSize(builder)); NodeBuilder allow = builder.child("allow"); allow.setProperty("jcr:primaryType", "rep:GrantACE"); allow.setProperty("rep:principalName", "administrators"); allow.setProperty(PropertyStates.createProperty( "rep:privileges", ImmutableList.of("jcr:all"), Type.NAMES)); - assertEquals(288, getSize(builder)); - assertEquals(72, getAmortizedSize(builder)); + assertEquals(320, getSize(builder)); + assertEquals(84, getAmortizedSize(builder)); NodeBuilder deny0 = builder.child("deny0"); deny0.setProperty("jcr:primaryType", "rep:DenyACE", Type.NAME); @@ -143,16 +143,16 @@ deny0.setProperty("rep:glob", "*/activities/*"); builder.setProperty(PropertyStates.createProperty( "rep:privileges", ImmutableList.of("jcr:read"), Type.NAMES)); - assertEquals(384, getSize(builder)); - assertEquals(108, getAmortizedSize(builder)); + assertEquals(416, getSize(builder)); + assertEquals(124, getAmortizedSize(builder)); NodeBuilder allow0 = builder.child("allow0"); allow0.setProperty("jcr:primaryType", "rep:GrantACE"); allow0.setProperty("rep:principalName", "user-administrators"); allow0.setProperty(PropertyStates.createProperty( "rep:privileges", ImmutableList.of("jcr:all"), Type.NAMES)); - assertEquals(432, getSize(builder)); - assertEquals(136, getAmortizedSize(builder)); + assertEquals(480, getSize(builder)); + assertEquals(160, getAmortizedSize(builder)); } @Test Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/LargeNumberOfPropertiesTestIT.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/LargeNumberOfPropertiesTestIT.java (revision 0) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/LargeNumberOfPropertiesTestIT.java (revision 0) @@ -0,0 +1,85 @@ +/* + * 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.plugins.segment.file; + +import java.io.File; +import java.io.IOException; + +import org.apache.commons.io.FileUtils; +import org.apache.jackrabbit.oak.plugins.segment.SegmentNodeStore; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; +import org.apache.jackrabbit.oak.spi.commit.EmptyHook; +import org.apache.jackrabbit.oak.spi.state.NodeBuilder; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class LargeNumberOfPropertiesTestIT { + + private static final Logger LOG = LoggerFactory + .getLogger(SegmentReferenceLimitTestIT.class); + + private File directory; + + @Before + public void setUp() throws IOException { + directory = File.createTempFile("LargeNumberOfPropertiesTestIT", "dir", + new File("target")); + directory.delete(); + directory.mkdir(); + } + + @After + public void cleanDir() { + try { + FileUtils.deleteDirectory(directory); + } catch (IOException e) { + LOG.error("Error cleaning directory", e); + } + } + + /** + * OAK-2481 IllegalStateException in TarMk with large number of properties + * + * @see OAK-2481 + */ + @Test + public void corruption() throws Exception { + FileStore fileStore = new FileStore(directory, 5, 0, false); + SegmentNodeStore nodeStore = new SegmentNodeStore(fileStore); + + NodeBuilder root = nodeStore.getRoot().builder(); + + try { + NodeBuilder c = root.child("c" + System.currentTimeMillis()); + // i=26 hits the hard limit for the number of properties a node can + // have (262144) + for (int i = 0; i < 25; i++) { + for (int j = 0; j < 10000; j++) { + c.setProperty("int-" + i + "-" + j, i); + } + } + nodeStore.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY); + } finally { + fileStore.close(); + } + } + +} Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/SegmentReferenceLimitTestIT.java =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/SegmentReferenceLimitTestIT.java (revision 1660375) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/SegmentReferenceLimitTestIT.java (working copy) @@ -43,13 +43,14 @@ import org.slf4j.LoggerFactory; public class SegmentReferenceLimitTestIT { + private static final Logger LOG = LoggerFactory.getLogger(SegmentReferenceLimitTestIT.class); private File directory; @Before public void setUp() throws IOException { - directory = File.createTempFile("FileStoreTest", "dir", new File("target")); + directory = File.createTempFile("SegmentReferenceLimitTestIT", "dir", new File("target")); directory.delete(); directory.mkdir(); } @@ -63,6 +64,12 @@ } } + /** + * OAK-2294 Corrupt repository after concurrent version operations + * + * @see OAK-2294 + */ @Test public void corruption() throws IOException, CommitFailedException, ExecutionException, InterruptedException { FileStore fileStore = new FileStore(directory, 1, 0, false); @@ -107,11 +114,12 @@ @Override public Void call() throws Exception { - for (int k = 0; ; k++) { + for (int k = 0; k < 1500; k++) { NodeBuilder root = nodeStore.getRoot().builder(); root.getChildNode("test").setProperty(name + ' ' + k, name + " value " + k); nodeStore.merge(root, EmptyHook.INSTANCE, CommitInfo.EMPTY); } + return null; } }