Index: oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriter.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriter.java (date 1430751088000) +++ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriter.java (date 1429525194000) @@ -20,7 +20,10 @@ import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkPositionIndexes; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Lists.reverse; import static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Maps.newLinkedHashMap; import static com.google.common.collect.Maps.newTreeMap; import static com.google.common.collect.Sets.newHashSet; import static org.apache.jackrabbit.oak.plugins.segment.Segment.REF_COUNT_OFFSET; @@ -132,7 +135,7 @@ * and finally by the {@link #close()} method to generate the tar index. * Should only be accessed from synchronized code; */ - private final Map index = newHashMap(); + private final Map index = newLinkedHashMap(); private final Set references = newHashSet(); @@ -447,15 +450,14 @@ * @throws IOException */ synchronized void collectReferences(Set referencedIds) throws IOException { - Set referenced = newHashSet(); - for (UUID id : referencedIds) { - List refs = graph.get(id); + for (UUID uuid : reverse(newArrayList(index.keySet()))) { + if (referencedIds.remove(uuid)) { + List refs = graph.get(uuid); - if (refs != null) { + if (refs != null) { - referenced.addAll(refs); + referencedIds.addAll(refs); - } - } + } + } - referencedIds.addAll(referenced); - referencedIds.removeAll(index.keySet()); + } } //------------------------------------------------------------< Object >-- Index: oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriterTest.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== --- oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriterTest.java (date 1429525194000) +++ oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/file/TarWriterTest.java (date 1429525194000) @@ -0,0 +1,173 @@ +/* + * 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 static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Sets.newHashSet; +import static java.io.File.createTempFile; +import static java.nio.ByteBuffer.allocate; +import static java.util.Collections.singleton; +import static org.apache.jackrabbit.oak.plugins.segment.SegmentVersion.V_11; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.Map; +import java.util.Set; +import java.util.UUID; + +import com.google.common.collect.ImmutableList; +import org.apache.jackrabbit.oak.plugins.segment.RecordId; +import org.apache.jackrabbit.oak.plugins.segment.Segment; +import org.apache.jackrabbit.oak.plugins.segment.SegmentId; +import org.apache.jackrabbit.oak.plugins.segment.SegmentStore; +import org.apache.jackrabbit.oak.plugins.segment.SegmentTracker; +import org.apache.jackrabbit.oak.plugins.segment.SegmentWriter; +import org.apache.jackrabbit.oak.plugins.segment.file.TarWriterTest.SegmentGraphBuilder.Node; +import org.apache.jackrabbit.oak.plugins.segment.memory.MemoryStore; +import org.junit.Test; + +public class TarWriterTest { + + /** + * Regression test for OAK-2800 + */ + @Test + public void collectReferences() throws IOException { + SegmentGraphBuilder graphBuilder = new SegmentGraphBuilder(); + + // a -> b -> c + Node c = graphBuilder.createNode("c"); + Node b = graphBuilder.createNode("b", c); + Node a = graphBuilder.createNode("a", b); + + // y -> z + Node z = graphBuilder.createNode("z"); + Node y = graphBuilder.createNode("y", z); + + assertEquals(singleton(b), a.getReferences()); + assertEquals(singleton(c), b.getReferences()); + assertTrue(c.getReferences().isEmpty()); + assertEquals(singleton(z), y.getReferences()); + assertTrue(z.getReferences().isEmpty()); + + File tar = createTempFile(getClass().getName(), "tar"); + TarWriter tarWriter = new TarWriter(tar); + try { + y.write(tarWriter); + b.write(tarWriter); + a.write(tarWriter); + + Set references = newHashSet(); + references.add(a.getUUID()); + tarWriter.collectReferences(references); + assertEquals(a + " must have exactly a reference to " + c, singleton(c.getUUID()), references); + } finally { + tarWriter.close(); + } + } + + public static class SegmentGraphBuilder { + private final Map segments = newHashMap(); + private final Map nodes = newHashMap(); + + private final SegmentStore store = new MemoryStore() { + @Override + public void writeSegment(SegmentId id, byte[] data, int offset, int length) { + super.writeSegment(id, data, offset, length); + ByteBuffer buffer = allocate(length); + buffer.put(data, offset, length); + buffer.rewind(); + segments.put(id, buffer); + } + }; + private final SegmentTracker tracker = new SegmentTracker(store, V_11); + private final SegmentWriter writer = new SegmentWriter(store, tracker, V_11); + + private int nextNodeNo; + + public class Node { + final String name; + final RecordId selfId; + final byte[] data; + final Segment segment; + + Node(String name, RecordId selfId, ByteBuffer data) { + this.name = name; + this.selfId = selfId; + this.data = data.array(); + segment = new Segment(tracker, selfId.getSegmentId(), data); + } + + public void write(TarWriter tarWriter) throws IOException { + long msb = getSegmentId().getMostSignificantBits(); + long lsb = getSegmentId().getLeastSignificantBits(); + tarWriter.writeEntry(msb, lsb, data, 0, data.length); + } + + public UUID getUUID() { + return newUUID(getSegmentId()); + } + + private SegmentId getSegmentId() { + return selfId.getSegmentId(); + } + + public Set getReferences() { + Set references = newHashSet(); + for (SegmentId segmentId : segment.getReferencedIds()) { + references.add(nodes.get(newUUID(segmentId))); + } + references.remove(this); + return references; + } + + @Override + public String toString() { + return name; + } + + void addReference(SegmentWriter writer) { + // Need to write a proper list as singleton lists are optimised + // to just returning the recordId of its single element + writer.writeList(ImmutableList.of(selfId, selfId)); + } + } + + public Node createNode(String name, Node... refs) { + RecordId selfId = writer.writeString("id-" + nextNodeNo++); + for (Node ref : refs) { + ref.addReference(writer); + } + writer.flush(); + SegmentId segmentId = selfId.getSegmentId(); + Node node = new Node(name, selfId, segments.get(segmentId)); + nodes.put(newUUID(segmentId), node); + return node; + } + + private static UUID newUUID(SegmentId segmentId) { + return new UUID(segmentId.getMostSignificantBits(), segmentId.getLeastSignificantBits()); + } + } + +}