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 ju...@apache.org on 2013/10/21 17:01:46 UTC
svn commit: r1534199 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/segment/
test/java/org/apache/jackrabbit/oak/plugins/segment/
Author: jukka
Date: Mon Oct 21 15:01:46 2013
New Revision: 1534199
URL: http://svn.apache.org/r1534199
Log:
OAK-1104: SegmentNodeStore rebase operation assumes wrong child node order
Use unsigned comparison of the hash values in MapRecord to avoid unexpected ordering
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/MapRecordTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapEntry.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapEntry.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapEntry.java?rev=1534199&r1=1534198&r2=1534199&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapEntry.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapEntry.java Mon Oct 21 15:01:46 2013
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.oak.plugin
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
+import static org.apache.jackrabbit.oak.plugins.segment.MapRecord.HASH_MASK;
import java.util.Map;
@@ -87,7 +88,7 @@ class MapEntry extends AbstractChildNode
@Override
public int compareTo(MapEntry that) {
return ComparisonChain.start()
- .compare(getHash(), that.getHash())
+ .compare(getHash() & HASH_MASK, that.getHash() & HASH_MASK)
.compare(name, that.name)
.compare(value, that.value)
.result();
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java?rev=1534199&r1=1534198&r2=1534199&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/MapRecord.java Mon Oct 21 15:01:46 2013
@@ -37,6 +37,7 @@ class MapRecord extends Record {
private static final long M = 0x5DEECE66DL;
private static final long A = 0xBL;
+ static final long HASH_MASK = 0xFFFFFFFFL;
static int getHash(String name) {
return (int) (((name.hashCode() ^ M) * M + A) >> 16);
@@ -153,7 +154,7 @@ class MapRecord extends Record {
int bitmap = segment.readInt(getOffset(4));
int mask = BUCKETS_PER_LEVEL - 1;
int shift = 32 - (level + 1) * LEVEL_BITS;
- int index = (hash >> shift) & mask;
+ int index = (int) (hash >> shift) & mask;
int bit = 1 << index;
if ((bitmap & bit) != 0) {
int ids = bitCount(bitmap & (bit - 1));
@@ -167,8 +168,8 @@ class MapRecord extends Record {
// this is a leaf record; scan the list to find a matching entry
int d = -1;
for (int i = 0; i < size && d < 0; i++) {
- d = Integer.valueOf(segment.readInt(getOffset(4 + i * 4)))
- .compareTo(Integer.valueOf(hash));
+ d = Long.valueOf(segment.readInt(getOffset(4 + i * 4)) & HASH_MASK)
+ .compareTo(Long.valueOf(hash & HASH_MASK));
if (d == 0) {
RecordId keyId = segment.readRecordId(
getOffset(4 + size * 4, i));
@@ -379,7 +380,7 @@ class MapRecord extends Record {
return -1; // see above
} else {
return ComparisonChain.start()
- .compare(before.getHash(), after.getHash())
+ .compare(before.getHash() & HASH_MASK, after.getHash() & HASH_MASK)
.compare(before.getName(), after.getName())
.result();
}
Added: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/MapRecordTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/MapRecordTest.java?rev=1534199&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/MapRecordTest.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/segment/MapRecordTest.java Mon Oct 21 15:01:46 2013
@@ -0,0 +1,92 @@
+/*
+ * 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;
+
+import static com.google.common.collect.Sets.newHashSet;
+import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE;
+import static org.easymock.EasyMock.createControl;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import org.apache.jackrabbit.oak.plugins.segment.memory.MemoryStore;
+import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
+import org.junit.Test;
+
+import com.google.common.collect.Sets;
+
+/**
+ * Test case for segment node state comparisons.
+ */
+public class MapRecordTest {
+
+ private final NodeStateDiff diff =
+ createControl().createMock("diff", NodeStateDiff.class);
+
+ private NodeBuilder builder =
+ new MemoryStore().getWriter().writeNode(EMPTY_NODE).builder();
+
+ @Test
+ public void testOak1104() {
+ Pattern pattern = Pattern.compile(", ");
+ Set<String> beforeNames = newHashSet(pattern.split(
+ "_b_Lucene41_0.doc, _b.fdx, _b.fdt, segments_34, _b_4.del,"
+ + " _b_Lucene41_0.pos, _b.nvm, _b.nvd, _b.fnm, _3n.si,"
+ + " _b_Lucene41_0.tip, _b_Lucene41_0.tim, _3n.cfe,"
+ + " segments.gen, _3n.cfs, _b.si"));
+ Set<String> afterNames = newHashSet(pattern.split(
+ "_b_Lucene41_0.pos, _3k.cfs, _3j_1.del, _b.nvm, _b.nvd,"
+ + " _3d.cfe, _3d.cfs, _b.fnm, _3j.si, _3h.si, _3i.cfe,"
+ + " _3i.cfs, _3e_2.del, _3f.si, _b_Lucene41_0.tip,"
+ + " _b_Lucene41_0.tim, segments.gen, _3e.cfe, _3e.cfs,"
+ + " _b.si, _3g.si, _3l.si, _3i_1.del, _3d_3.del, _3e.si,"
+ + " _3d.si, _b_Lucene41_0.doc, _3h_2.del, _3i.si, _3k_1.del,"
+ + " _3j.cfe, _3j.cfs, _b.fdx, _b.fdt, _3g_1.del, _3k.si,"
+ + " _3l.cfe, _3l.cfs, segments_33, _3f_1.del, _3h.cfe,"
+ + " _3h.cfs, _b_4.del, _3f.cfe, _3f.cfs, _3g.cfe, _3g.cfs"));
+
+ for (String name : beforeNames) {
+ builder.setChildNode(name);
+ }
+ NodeState before = builder.getNodeState();
+
+ for (String name : Sets.difference(beforeNames, afterNames)) {
+ builder.getChildNode(name).remove();
+ }
+ for (String name : Sets.difference(afterNames, beforeNames)) {
+ builder.setChildNode(name);
+ }
+ NodeState after = builder.getNodeState();
+
+ for (String name : Sets.difference(beforeNames, afterNames)) {
+ expect(diff.childNodeDeleted(name, before.getChildNode(name))).andReturn(true);
+ }
+ for (String name : Sets.difference(afterNames, beforeNames)) {
+ expect(diff.childNodeAdded(name, after.getChildNode(name))).andReturn(true);
+ }
+ replay(diff);
+
+ after.compareAgainstBaseState(before, diff);
+ verify(diff);
+ }
+
+}