dnhatn commented on a change in pull request #1427: LUCENE-9304: Fix
IW#getMaxCompletedSequenceNumber()
URL: https://github.com/apache/lucene-solr/pull/1427#discussion_r407865438
##########
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:
Should `() -> nextSeqNo.get() - 1` be `this:: getMaxCompletedSeqNo` instead?
----------------------------------------------------------------
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
