s1monw commented on a change in pull request #1427: LUCENE-9304: Fix IW#getMaxCompletedSequenceNumber() URL: https://github.com/apache/lucene-solr/pull/1427#discussion_r407988152
########## File path: lucene/core/src/java/org/apache/lucene/index/DocumentsWriterDeleteQueue.java ########## @@ -538,17 +546,65 @@ public String toString() { public long getNextSequenceNumber() { long seqNo = nextSeqNo.getAndIncrement(); - assert seqNo < maxSeqNo: "seqNo=" + seqNo + " vs maxSeqNo=" + maxSeqNo; + assert seqNo <= maxSeqNo: "seqNo=" + seqNo + " vs maxSeqNo=" + maxSeqNo; return seqNo; } - public long getLastSequenceNumber() { + long getLastSequenceNumber() { return nextSeqNo.get()-1; } /** Inserts a gap in the sequence numbers. This is used by IW during flush or commit to ensure any in-flight threads get sequence numbers * inside the gap */ - public void skipSequenceNumbers(long jump) { + void skipSequenceNumbers(long jump) { nextSeqNo.addAndGet(jump); - } + } + + /** + * Returns the maximum completed seq no for this queue. + */ + long getMaxCompletedSeqNo() { + if (startSeqNo < nextSeqNo.get()) { + return getLastSequenceNumber(); + } else { + // if we haven't advanced the seqNo make sure we fall back to the previous queue + long value = previousMaxSeqId.getAsLong(); + assert value <= startSeqNo : "illegal max sequence ID: " + value + " start was: " + startSeqNo; + return value; + } + } + + /** + * Advances the queue to the next queue on flush. This carries over the the generation to the next queue and + * set the {@link #getMaxSeqNo()} based on the given maxNumPendingOps. This method can only be called once, subsequently + * the returned queue should be used. + * @param maxNumPendingOps the max number of possible concurrent operations that will execute on this queue after + * it was advanced. This corresponds the the number of DWPTs that own the current queue at the + * moment when this queue is advanced since each these DWPTs can increment the seqId after we + * advanced it. + * @return a new queue as a successor of this queue. + */ + synchronized DocumentsWriterDeleteQueue advanceQueue(int maxNumPendingOps) { + if (advanced) { + throw new IllegalStateException("queue was already advanced"); + } + advanced = true; + long seqNo = getLastSequenceNumber() + maxNumPendingOps + 1; + maxSeqNo = seqNo; + return new DocumentsWriterDeleteQueue(infoStream, generation + 1, seqNo + 1, () -> nextSeqNo.get() - 1); Review comment: I left a comment why this is not a good idea. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org