Skip to content

Commit 5e7cc07

Browse files
perf: store a unique index as one id per key instead of a one-element list
A unique index kept the classic value -> [id] layout: a CopyOnWriteArrayList per key that never held more than one element, allocated and copied on every write, plus a size check standing in for the uniqueness test. The index now stores the id itself (value -> id) in a map of its own, named with a "|unique" suffix, and enforces uniqueness by comparing the stored id with the writer's: another document under the key is a violation, the same document again is not. An index still in the list layout is migrated the first time it is accessed and the legacy map is dropped, the way the composite layout migrates a non-unique index. drop() removes whichever layouts exist without migrating first. IndexMap exposes the single-id map to the scanner and the filters as one-element lists, so the read path is unchanged; readSortKeys reads the pairs directly. The map has its own name because the RocksDB adapter decodes values by the declared type of the map they live in. IndexManager.close(), clearAll() and dropIndexDescriptor() acted only on the map name recorded in IndexMeta, which is the classic one. That already missed the composite map of a non-unique index: after collection.clear() its rows survived, and a query on that index returned the ids of the cleared documents alongside the new ones (two live documents, four results). With the unique layout it would have rejected the very keys the collection no longer held. All three now cover every layout map an index can occupy. Tests cover write, violation, same-document rewrite, remove by the right and the wrong id, migration from the list layout, drop of both layouts, sort keys, and clear() followed by fresh documents under the same keys on both a non-unique and a unique index. The core and RocksDB suites pass. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 38caf34 commit 5e7cc07

7 files changed

Lines changed: 263 additions & 52 deletions

File tree

‎nitrite/src/main/java/org/dizitart/no2/collection/operation/IndexManager.java‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@
2828
import java.util.*;
2929
import java.util.concurrent.atomic.AtomicBoolean;
3030

31+
import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName;
3132
import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMapName;
33+
import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName;
3234
import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMetaMapName;
3335

3436
/**
@@ -93,32 +95,57 @@ public void close() {
9395
Iterable<IndexMeta> indexMetas = indexMetaMap.values();
9496
for (IndexMeta indexMeta : indexMetas) {
9597
if (indexMeta != null && indexMeta.getIndexDescriptor() != null) {
96-
String indexMapName = indexMeta.getIndexMap();
97-
NitriteMap<?, ?> indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class);
98-
indexMap.close();
98+
for (NitriteMap<?, ?> indexMap : existingLayoutMaps(indexMeta)) {
99+
indexMap.close();
100+
}
99101
}
100102
}
101-
102103
// close index meta
103104
indexMetaMap.close();
104105
}
105106
}
106107

107108
public void clearAll() {
108-
// close all index maps
109+
// clear and close all index maps
109110
if (!indexMetaMap.isClosed() && !indexMetaMap.isDropped()) {
110111
Iterable<IndexMeta> indexMetas = indexMetaMap.values();
111112
for (IndexMeta indexMeta : indexMetas) {
112113
if (indexMeta != null && indexMeta.getIndexDescriptor() != null) {
113-
String indexMapName = indexMeta.getIndexMap();
114-
NitriteMap<?, ?> indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class);
115-
indexMap.clear();
116-
indexMap.close();
114+
for (NitriteMap<?, ?> indexMap : existingLayoutMaps(indexMeta)) {
115+
indexMap.clear();
116+
indexMap.close();
117+
}
117118
}
118119
}
119120
}
120121
}
121122

123+
/**
124+
* The maps an index actually occupies in the store. {@link IndexMeta#getIndexMap()} records
125+
* the classic map name, but a single-field index may instead live in the composite layout
126+
* (non-unique) or the single-id layout (unique), each under a derived name of its own, and
127+
* an index in mid-migration can briefly have two. Closing, clearing or dropping only the
128+
* recorded map leaves the real one behind: after {@code clear()} its stale entries resolve
129+
* to deleted documents, and a unique index rejects the very keys the collection no longer
130+
* holds.
131+
*/
132+
private List<NitriteMap<?, ?>> existingLayoutMaps(IndexMeta indexMeta) {
133+
List<String> names = new ArrayList<>();
134+
names.add(indexMeta.getIndexMap());
135+
IndexDescriptor descriptor = indexMeta.getIndexDescriptor();
136+
if (!descriptor.isCompoundIndex()) {
137+
names.add(deriveCompositeIndexMapName(descriptor));
138+
names.add(deriveUniqueIndexMapName(descriptor));
139+
}
140+
List<NitriteMap<?, ?>> maps = new ArrayList<>();
141+
for (String name : names) {
142+
if (nitriteStore.hasMap(name)) {
143+
maps.add(nitriteStore.openMap(name, Object.class, Object.class));
144+
}
145+
}
146+
return maps;
147+
}
148+
122149
/**
123150
* Is dirty index boolean.
124151
*
@@ -174,11 +201,10 @@ IndexDescriptor createIndexDescriptor(Fields fields, String indexType) {
174201
void dropIndexDescriptor(Fields fields) {
175202
IndexMeta meta = indexMetaMap.get(fields);
176203
if (meta != null && meta.getIndexDescriptor() != null) {
177-
String indexMapName = meta.getIndexMap();
178-
NitriteMap<?, ?> indexMap = nitriteStore.openMap(indexMapName, Object.class, Object.class);
179-
indexMap.drop();
204+
for (NitriteMap<?, ?> indexMap : existingLayoutMaps(meta)) {
205+
indexMap.drop();
206+
}
180207
}
181-
182208
indexMetaMap.remove(fields);
183209
updateIndexDescriptorCache();
184210
}

‎nitrite/src/main/java/org/dizitart/no2/common/util/IndexUtils.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ public static String deriveCompositeIndexMapName(IndexDescriptor descriptor) {
4848
return deriveIndexMapName(descriptor) + INTERNAL_NAME_SEPARATOR + "composite";
4949
}
5050

51+
/**
52+
* Derives the name of the map holding a unique index in its single-id layout, one
53+
* {@code value -> id} entry per key.
54+
*
55+
* @param descriptor the index descriptor
56+
* @return the map name
57+
*/
58+
public static String deriveUniqueIndexMapName(IndexDescriptor descriptor) {
59+
return deriveIndexMapName(descriptor) + INTERNAL_NAME_SEPARATOR + "unique";
60+
}
61+
5162
public static String deriveIndexMetaMapName(String collectionName) {
5263
return INDEX_META_PREFIX + INTERNAL_NAME_SEPARATOR + collectionName;
5364
}

‎nitrite/src/main/java/org/dizitart/no2/index/IndexMap.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public class IndexMap {
4040
// (value, id) pairs (see IndexEntryKey). This IndexMap still presents the classic
4141
// value -> List<NitriteId> view to the scanner and the filters.
4242
private NitriteMap<IndexEntryKey, ?> compositeMap;
43+
// single-id layout (unique index): values are NitriteIds, exposed as one-element lists
44+
private boolean singleValued;
4345

4446
@Getter
4547
@Setter
@@ -79,6 +81,24 @@ public static IndexMap composite(NitriteMap<IndexEntryKey, ?> compositeMap) {
7981
return new IndexMap(compositeMap, true);
8082
}
8183

84+
/**
85+
* Instantiates an {@link IndexMap} over a unique index stored in the single-id layout
86+
* ({@code value -> id}). The scanner and the filters expect a list of ids under every key,
87+
* so each stored id is handed out as a one-element list.
88+
*
89+
* @param uniqueMap the backing map
90+
* @return the index map
91+
*/
92+
public static IndexMap unique(NitriteMap<DBValue, NitriteId> uniqueMap) {
93+
IndexMap indexMap = new IndexMap(uniqueMap);
94+
indexMap.singleValued = true;
95+
return indexMap;
96+
}
97+
98+
private static Object exposeValue(Object value, boolean singleValued) {
99+
return singleValued && value instanceof NitriteId ? Collections.singletonList(value) : value;
100+
}
101+
82102
/**
83103
* Normalizes a key returned by the backing map to the {@link DBNull} singleton
84104
* when it represents the null key. Persistent stores deserialize the stored null
@@ -228,7 +248,7 @@ public Object get(DBValue dbValue) {
228248
return compositeGet(dbValue == null ? DBNull.getInstance() : dbValue);
229249
}
230250
if (nitriteMap != null) {
231-
return nitriteMap.get(dbValue);
251+
return exposeValue(nitriteMap.get(dbValue), singleValued);
232252
} else if (navigableMap != null) {
233253
return navigableMap.get(dbValue);
234254
}
@@ -284,10 +304,11 @@ public boolean hasNext() {
284304
public Pair<DBValue, ?> next() {
285305
Pair<DBValue, ?> next = entryIterator.next();
286306
DBValue dbKey = next.getFirst();
307+
Object value = exposeValue(next.getSecond(), singleValued);
287308
if (dbKey instanceof DBNull) {
288-
return new Pair<>(null, next.getSecond());
309+
return new Pair<>(null, value);
289310
} else {
290-
return new Pair<>(dbKey, next.getSecond());
311+
return new Pair<>(dbKey, value);
291312
}
292313
}
293314
};

‎nitrite/src/main/java/org/dizitart/no2/index/NitriteIndex.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,8 +132,8 @@ default List<NitriteId> addNitriteIds(List<NitriteId> nitriteIds, FieldValues fi
132132
// ConcurrentModificationException. CopyOnWriteArrayList swaps its backing array
133133
// atomically on each mutation, so the background serializer always sees a stable
134134
// snapshot. Non-unique indexes avoid list values entirely via the composite layout
135-
// (issue #1260); only unique indexes and the text index reach this path, where the
136-
// per-key list is small enough that copy-on-write cost is negligible.
135+
// (issue #1260) and unique indexes store their single id directly; only the text
136+
// index still reaches this path.
137137
nitriteIds = new CopyOnWriteArrayList<>();
138138
}
139139

‎nitrite/src/main/java/org/dizitart/no2/index/SingleFieldIndex.java‎

Lines changed: 67 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.dizitart.no2.common.Fields;
2727
import org.dizitart.no2.common.tuples.Pair;
2828
import org.dizitart.no2.filters.ComparableFilter;
29+
import org.dizitart.no2.exceptions.UniqueConstraintException;
2930
import org.dizitart.no2.store.NitriteMap;
3031
import org.dizitart.no2.store.NitriteStore;
3132

@@ -37,6 +38,7 @@
3738

3839
import static org.dizitart.no2.common.util.IndexUtils.deriveCompositeIndexMapName;
3940
import static org.dizitart.no2.common.util.IndexUtils.deriveIndexMapName;
41+
import static org.dizitart.no2.common.util.IndexUtils.deriveUniqueIndexMapName;
4042
import static org.dizitart.no2.common.util.ObjectUtils.convertToObjectArray;
4143

4244
/**
@@ -48,6 +50,7 @@ public class SingleFieldIndex implements NitriteIndex {
4850
private final IndexDescriptor indexDescriptor;
4951
private final NitriteStore<?> nitriteStore;
5052
private volatile boolean migrationChecked;
53+
private volatile boolean uniqueMigrationChecked;
5154

5255
/**
5356
* Instantiates a new {@link SingleFieldIndex}.
@@ -61,9 +64,9 @@ public SingleFieldIndex(IndexDescriptor indexDescriptor, NitriteStore<?> nitrite
6164
}
6265

6366
/**
64-
* The composite-key layout (issue #1260) is used for every non-unique index. Unique indexes
65-
* keep the classic {@code value -> [id]} array layout because the uniqueness check relies on
66-
* its single-array shape.
67+
* The composite-key layout (issue #1260) is used for every non-unique index. A unique index
68+
* has at most one id per key, so it stores that id directly ({@code value -> id}); the
69+
* classic {@code value -> [id]} list layout it used before is migrated on first use.
6770
*/
6871
private boolean useCompositeLayout() {
6972
return !isUnique();
@@ -78,10 +81,15 @@ public void write(FieldValues fieldValues) {
7881
Object element = fieldValues.get(firstField);
7982

8083
if (!useCompositeLayout()) {
81-
// unique indexes (and stores without comparable key ordering) keep the classic
82-
// value -> [id] layout.
83-
NitriteMap<DBValue, List<?>> indexMap = findIndexMap();
84-
forEachElement(element, dbValue -> addIndexElement(indexMap, fieldValues, dbValue));
84+
// one id per key: a violation is another document already holding the key
85+
NitriteMap<DBValue, NitriteId> indexMap = findUniqueMap();
86+
forEachElement(element, dbValue -> {
87+
NitriteId existing = indexMap.get(dbValue);
88+
if (existing != null && !existing.equals(fieldValues.getNitriteId())) {
89+
throw new UniqueConstraintException("Unique key constraint violation for " + fields);
90+
}
91+
indexMap.put(dbValue, fieldValues.getNitriteId());
92+
});
8593
} else {
8694
// non-unique indexes use the composite-key layout: one O(log n) point write per
8795
// (value, id) pair, instead of an O(n) read-modify-write of a shared list (issue #1260)
@@ -100,8 +108,13 @@ public void remove(FieldValues fieldValues) {
100108
Object element = fieldValues.get(firstField);
101109

102110
if (!useCompositeLayout()) {
103-
NitriteMap<DBValue, List<?>> indexMap = findIndexMap();
104-
forEachElement(element, dbValue -> removeIndexElement(indexMap, fieldValues, dbValue));
111+
NitriteMap<DBValue, NitriteId> indexMap = findUniqueMap();
112+
forEachElement(element, dbValue -> {
113+
NitriteId existing = indexMap.get(dbValue);
114+
if (existing != null && existing.equals(fieldValues.getNitriteId())) {
115+
indexMap.remove(dbValue);
116+
}
117+
});
105118
} else {
106119
NitriteMap<IndexEntryKey, Object> indexMap = findCompositeMap();
107120
forEachElement(element, dbValue ->
@@ -112,9 +125,10 @@ public void remove(FieldValues fieldValues) {
112125
@Override
113126
public void drop() {
114127
if (!useCompositeLayout()) {
115-
NitriteMap<DBValue, List<?>> indexMap = findIndexMap();
116-
indexMap.clear();
117-
indexMap.drop();
128+
// drop whichever layouts exist without migrating first; nothing being dropped
129+
// needs converting
130+
dropMapIfPresent(deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class);
131+
dropMapIfPresent(deriveIndexMapName(indexDescriptor), DBValue.class, ArrayList.class);
118132
} else {
119133
NitriteMap<IndexEntryKey, Object> indexMap = findCompositeMap();
120134
indexMap.clear();
@@ -129,7 +143,7 @@ public LinkedHashSet<NitriteId> findNitriteIds(FindPlan findPlan) {
129143

130144
IndexMap iMap = useCompositeLayout()
131145
? IndexMap.composite(findCompositeMap())
132-
: new IndexMap(findIndexMap());
146+
: IndexMap.unique(findUniqueMap());
133147
return scanIndex(findPlan, iMap);
134148
}
135149

@@ -147,11 +161,9 @@ public List<Pair<DBValue, NitriteId>> readSortKeys(long collectionSize) {
147161
keys.add(new Pair<>(key.getValue(), key.getNitriteId()));
148162
}
149163
} else {
150-
for (Pair<DBValue, List<?>> entry : (Iterable<Pair<DBValue, List<?>>>) (Iterable<?>) findIndexMap().entries()) {
151-
for (NitriteId nitriteId : (List<NitriteId>) entry.getSecond()) {
152-
if (!seen.add(nitriteId)) return null;
153-
keys.add(new Pair<>(entry.getFirst(), nitriteId));
154-
}
164+
for (Pair<DBValue, NitriteId> entry : findUniqueMap().entries()) {
165+
if (!seen.add(entry.getSecond())) return null;
166+
keys.add(new Pair<>(entry.getFirst(), entry.getSecond()));
155167
}
156168
}
157169

@@ -180,25 +192,47 @@ private void forEachElement(Object element, java.util.function.Consumer<DBValue>
180192
}
181193
}
182194

183-
@SuppressWarnings("unchecked")
184-
private void addIndexElement(NitriteMap<DBValue, List<?>> indexMap,
185-
FieldValues fieldValues, DBValue element) {
186-
List<NitriteId> nitriteIds = (List<NitriteId>) indexMap.get(element);
187-
nitriteIds = addNitriteIds(nitriteIds, fieldValues);
188-
indexMap.put(element, nitriteIds);
195+
private NitriteMap<DBValue, NitriteId> findUniqueMap() {
196+
migrateLegacyUniqueIndex();
197+
return nitriteStore.openMap(deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class);
189198
}
190199

200+
/**
201+
* Rewrites a unique index left in the classic {@code value -> [id]} list layout into the
202+
* single-id layout the first time the index is accessed, then drops the legacy map. The
203+
* list layout paid a copy-on-write list per key for a list that never held more than one
204+
* id. Idempotent and run once per index instance.
205+
*/
191206
@SuppressWarnings("unchecked")
192-
private void removeIndexElement(NitriteMap<DBValue, List<?>> indexMap,
193-
FieldValues fieldValues, DBValue element) {
194-
List<NitriteId> nitriteIds = (List<NitriteId>) indexMap.get(element);
195-
if (nitriteIds != null && !nitriteIds.isEmpty()) {
196-
nitriteIds.remove(fieldValues.getNitriteId());
197-
if (nitriteIds.size() == 0) {
198-
indexMap.remove(element);
199-
} else {
200-
indexMap.put(element, nitriteIds);
207+
private void migrateLegacyUniqueIndex() {
208+
if (uniqueMigrationChecked) return;
209+
synchronized (this) {
210+
if (uniqueMigrationChecked) return;
211+
String legacyName = deriveIndexMapName(indexDescriptor);
212+
if (nitriteStore.hasMap(legacyName)) {
213+
NitriteMap<DBValue, List<?>> legacy = findIndexMap();
214+
if (!legacy.isEmpty()) {
215+
NitriteMap<DBValue, NitriteId> unique = nitriteStore.openMap(
216+
deriveUniqueIndexMapName(indexDescriptor), DBValue.class, NitriteId.class);
217+
for (Pair<DBValue, List<?>> entry : (Iterable<Pair<DBValue, List<?>>>) (Iterable<?>) legacy.entries()) {
218+
List<NitriteId> nitriteIds = (List<NitriteId>) entry.getSecond();
219+
if (nitriteIds != null && !nitriteIds.isEmpty()) {
220+
unique.put(entry.getFirst(), nitriteIds.get(0));
221+
}
222+
}
223+
}
224+
legacy.clear();
225+
legacy.drop();
201226
}
227+
uniqueMigrationChecked = true;
228+
}
229+
}
230+
231+
private void dropMapIfPresent(String mapName, Class<?> keyType, Class<?> valueType) {
232+
if (nitriteStore.hasMap(mapName)) {
233+
NitriteMap<?, ?> map = nitriteStore.openMap(mapName, keyType, valueType);
234+
map.clear();
235+
map.drop();
202236
}
203237
}
204238

0 commit comments

Comments
 (0)