An index-entry-limit of 0 is treated as the smallest limit when a change of the limit decides whether the index must be rebuilt

[vharseko]

(line numbers on master, 129fc4e)

Describe the bug

index-entry-limit accepts 0, and the property says what it means: "For no limit, use 0 for the value"
(BackendIndexConfiguration.xml:62, lower-limit="0"; the backend-wide property every index inherits by default says
the same, PluggableBackendConfiguration.xml:256). The live index honours that — DefaultIndex.computeEntryIDSet
gives a key up only while indexEntryLimit > 0 (DefaultIndex.java:247), and the search cursor limit does too
(EntryContainer.getEntryIDSetLimit:1000). Three other readers of the same value compare it as an ordinary number,
and for all three 0 is then the smallest limit rather than none.

1. The decision whether a changed limit needs a rebuild — fix: a second PR on top of #997, once #997 is merged

DefaultIndex.setIndexEntryLimit (:302-307) answers "the limit went up" with a plain comparison:

final boolean rebuildRequired = this.indexEntryLimit < indexEntryLimit;

and AttributeIndex.updateIndex (:1000) untrusts the index and asks for a rebuild on that answer. So:

• 4000 → 0: 4000 < 0 is false — no rebuild, the index stays trusted. Every key which reached 4000 entries
  is undefined and stays undefined, while the configuration now promises that no key is given up. A search on
  such a key is answered from the undefined set as before, but nothing tells the operator that the index does not
  hold what its configuration declares.
• 0 → 4000: 0 < 4000 is true — the index is untrusted and a rebuild is demanded, though a lowered limit
  never invalidates a key (the comment above the comparison says so: "a new smaller index size limit doesn't
  impact validity of the results"). The index goes out of use until a rebuild which changes nothing.

2. Import and rebuild write every key undefined — fix: #1060

OnDiskMergeImporter.EntryIDsCollector (phase one, :3579/3593) and EntryIDSetsCollector (phase two,
:3654/3668/3688) read index.getIndexEntryLimit() and compare resultContainer.size() < indexLimit /
>= indexLimit. With a limit of 0, accept never accepts (0 < 0) and merge always answers the undefined set
(0 >= 0) — the one-chunk fast path of phase two sits behind the same comparison. Every key an import-ldif or a
rebuild-index writes under index-entry-limit: 0 is undefined, and afterPhaseTwo then marks the index trusted.

Reproduced against the collectors on master (DefaultIndex with the given limit, one key, one or two entry IDs):

limit=0    phase1(1 id) -> UNDEFINED        phase2(2 chunks) -> UNDEFINED        phase2(1 chunk) -> UNDEFINED
limit=4000 phase1(1 id) -> defined(size=1)  phase2(2 chunks) -> defined(size=2)  phase2(1 chunk) -> defined(size=1)

The effect is the opposite of "no limit": with every key undefined, every search which relies on that index is
unindexed, and a client without the unindexed-search privilege is refused with INSUFFICIENT_ACCESS_RIGHTS
(EntryContainer.search:975). Set on the backend (set-backend-prop --set index-entry-limit:0), the value is
inherited by every index, so one import puts the whole backend out of indexed use. The rebuild path is the same
code (RebuildIndexStrategy.newExternalSortChunk), so a rebuild cannot repair it either.

This is a regression of the importer rewrite (OPENDJ-2016, 641e89e, 2015): the importer it replaced handled
the value — ImportIDSet:78 used limit == 0 ? Integer.MAX_VALUE : limit and IndexKey:2352
entryLimit > 0 ? entryLimit : Integer.MAX_VALUE.

3. backendstat show-index-status — fix: #1060

BackendStat.appendIndexStats (:1139-1147) buckets every defined key by entryIDSet.size() >= entryLimit * 0.8
and so on. With a limit of 0 every key with at least one entry lands in the "over 95%" column.

Expected behavior

• A rebuild is required exactly when keys the index gave up may now have to be held: the old limit was bounded
  and the new one is either unbounded or higher.

  final boolean newLimitRequiresRebuild = oldLimit != 0 && (newLimit == 0 || oldLimit < newLimit);

• Import and rebuild give no key up under a limit of 0, as the importer before OPENDJ-2016 did and as the live
  index does.

• backendstat reports no "near the limit" figures for an index which has none.

Where

• OnDiskMergeImporter.EntryIDsCollector:3567 and EntryIDSetsCollector:3641 (the limit each one keeps) — [#1059] Give no key up on import or rebuild under an index-entry-limit of 0 #1060;
• BackendStat.appendIndexStats:1106 — [#1059] Give no key up on import or rebuild under an index-entry-limit of 0 #1060;
• DefaultIndex.setIndexEntryLimit:304; [#991] Decide and report an index configuration change outside the write which is replayed #997 moves the decision to AttributeIndex.planIndexUpdates and copies
  the comparison as it is, so the fix lands there once [#991] Decide and report an index configuration change outside the write which is replayed #997 is merged.
  ReplayedConfigChangeTest.aLoweredEntryLimitNeedsNoRebuildAndOpensNoTransaction from [#991] Decide and report an index configuration change outside the write which is replayed #997 is the case to
  extend with 4000 → 0 and 0 → 4000.

Order of the fixes

Today's wrong answer for 0 → 4000 is what repairs an index which an import wrote all-undefined: raising the limit
forces the rebuild that puts the keys back. A fix of the decision alone would leave such an index trusted and
undefined for good. So the importer is fixed first (or together), and indexes imported or rebuilt under
index-entry-limit: 0 before that fix have to be rebuilt once by hand.

Origin

Raised by @maximthomas in the review of #997. The importer and backendstat parts were found while verifying it.