You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/04/21 11:52:59 UTC
[GitHub] [lucene] mikemccand commented on a diff in pull request #633: LUCENE-10216: Use MergeScheduler and MergePolicy to run addIndexes(CodecReader[]) merges.
mikemccand commented on code in PR #633:
URL: https://github.com/apache/lucene/pull/633#discussion_r830147648
##########
lucene/core/src/java/org/apache/lucene/index/IndexWriter.java:
##########
@@ -3121,147 +3125,265 @@ private void validateMergeReader(CodecReader leaf) {
*/
public long addIndexes(CodecReader... readers) throws IOException {
ensureOpen();
-
- // long so we can detect int overflow:
- long numDocs = 0;
long seqNo;
- try {
- if (infoStream.isEnabled("IW")) {
- infoStream.message("IW", "flush at addIndexes(CodecReader...)");
- }
- flush(false, true);
+ long numDocs = 0;
+ final int mergeTimeoutInSeconds = 600;
- String mergedName = newSegmentName();
- int numSoftDeleted = 0;
- for (CodecReader leaf : readers) {
- numDocs += leaf.numDocs();
+ try {
+ // Best effort up front validations
+ for (CodecReader leaf: readers) {
validateMergeReader(leaf);
- if (softDeletesEnabled) {
- Bits liveDocs = leaf.getLiveDocs();
- numSoftDeleted +=
- PendingSoftDeletes.countSoftDeletes(
- DocValuesFieldExistsQuery.getDocValuesDocIdSetIterator(
- config.getSoftDeletesField(), leaf),
- liveDocs);
+ for (FieldInfo fi: leaf.getFieldInfos()) {
+ globalFieldNumberMap.verifyFieldInfo(fi);
}
+ numDocs += leaf.numDocs();
}
-
- // Best-effort up front check:
testReserveDocs(numDocs);
- final IOContext context =
- new IOContext(
- new MergeInfo(Math.toIntExact(numDocs), -1, false, UNBOUNDED_MAX_MERGE_SEGMENTS));
+ synchronized (this) {
+ ensureOpen();
+ if (merges.areEnabled() == false) {
+ throw new UnsupportedOperationException("Merges are disabled on current writer. " +
+ "Cannot execute addIndexes(CodecReaders...) API");
+ }
+ }
+
+ MergePolicy mergePolicy = config.getMergePolicy();
+ MergePolicy.MergeSpecification spec = mergePolicy.findMerges(Arrays.asList(readers));
Review Comment:
> We allow for cascading to proceed in background by calling a maybeMerge() at the end of addIndexes().
OK I think that's fair -- after `addIndexes` returns, the added segments are all copied into the target index, but (background) merges may need to still kick off and run to completion to get to a balanced index. Let's just be sure to advertise this in the javadocs?
##########
lucene/core/src/java/org/apache/lucene/index/IndexWriter.java:
##########
@@ -3127,143 +3135,307 @@ public long addIndexes(CodecReader... readers) throws IOException {
// long so we can detect int overflow:
long numDocs = 0;
long seqNo;
- try {
- if (infoStream.isEnabled("IW")) {
- infoStream.message("IW", "flush at addIndexes(CodecReader...)");
- }
- flush(false, true);
- String mergedName = newSegmentName();
- int numSoftDeleted = 0;
+ try {
+ // Best effort up front validations
for (CodecReader leaf : readers) {
- numDocs += leaf.numDocs();
validateMergeReader(leaf);
- if (softDeletesEnabled) {
- Bits liveDocs = leaf.getLiveDocs();
- numSoftDeleted +=
- PendingSoftDeletes.countSoftDeletes(
- DocValuesFieldExistsQuery.getDocValuesDocIdSetIterator(
- config.getSoftDeletesField(), leaf),
- liveDocs);
+ for (FieldInfo fi : leaf.getFieldInfos()) {
+ globalFieldNumberMap.verifyFieldInfo(fi);
}
+ numDocs += leaf.numDocs();
}
-
- // Best-effort up front check:
testReserveDocs(numDocs);
- final IOContext context =
- new IOContext(
- new MergeInfo(Math.toIntExact(numDocs), -1, false, UNBOUNDED_MAX_MERGE_SEGMENTS));
-
- // TODO: somehow we should fix this merge so it's
- // abortable so that IW.close(false) is able to stop it
- TrackingDirectoryWrapper trackingDir = new TrackingDirectoryWrapper(directory);
- Codec codec = config.getCodec();
- // We set the min version to null for now, it will be set later by SegmentMerger
- SegmentInfo info =
- new SegmentInfo(
- directoryOrig,
- Version.LATEST,
- null,
- mergedName,
- -1,
- false,
- codec,
- Collections.emptyMap(),
- StringHelper.randomId(),
- Collections.emptyMap(),
- config.getIndexSort());
-
- SegmentMerger merger =
- new SegmentMerger(
- Arrays.asList(readers), info, infoStream, trackingDir, globalFieldNumberMap, context);
+ synchronized (this) {
+ ensureOpen();
+ if (merges.areEnabled() == false) {
+ throw new AlreadyClosedException(
+ "Merges are disabled on current writer. "
Review Comment:
Can we reword this? The Lucene user will not know what `Merges are disabled...` means -- they did not ask for that to happen :) I think it means this `IW` is being concurrently closed or aborted (due to unexpected `OOME` maybe)?
##########
lucene/core/src/java/org/apache/lucene/index/MergePolicy.java:
##########
@@ -813,12 +866,24 @@ protected final boolean verbose(MergeContext mergeContext) {
}
static final class MergeReader {
+ final CodecReader codecReader;
final SegmentReader reader;
Review Comment:
OK let's leave it as is for now -- we can improve that later?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org