You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by s1monw <gi...@git.apache.org> on 2018/11/21 09:31:11 UTC

[GitHub] lucene-solr pull request #503: LUCENE-8571: Don't block on FrozenBufferedUpd...

GitHub user s1monw opened a pull request:

    https://github.com/apache/lucene-solr/pull/503

    LUCENE-8571: Don't block on FrozenBufferedUpdates#apply during IW#processEvents

    While indexing we try to apply frozen deletes packages concurrently
    on indexing threads if necessary. This is done in an opaque way via
    IndexWriter#processEvents. Yet, when we commit or refresh we have to
    ensure we apply all frozen update packages before we return.
    Today we execute the apply method in a blocking fashion which is unnecessary
    when we are in a IndexWriter#processEvents context, we block indexing
    threads while they could just continue since it's already being applied.
    We also might wait in BufferedUpdatesStream when we apply all necessary updates
    were we can continue with other work instead of waiting.
    This change also tries to apply the packages that are not currently applied
    first in order to not unnecessarily block.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/s1monw/lucene-solr optimize_apply_frozen_updates

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/lucene-solr/pull/503.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #503
    
----
commit caeb68509a53766963bbcb1a96ef204ba6cb1492
Author: Simon Willnauer <si...@...>
Date:   2018-11-21T09:22:41Z

    LUCENE-8571: Don't block on FrozenBufferedUpdates#apply during IW#processEvents
    
    While indexing we try to apply frozen deletes packages concurrently
    on indexing threads if necessary. This is done in an opaque way via
    IndexWriter#processEvents. Yet, when we commit or refresh we have to
    ensure we apply all frozen update packages before we return.
    Today we execute the apply method in a blocking fashion which is unncessary
    when we are in a IndexWriter#processEvents context, we block indexing
    threads while they could just continue since it's already being applied.
    We also might wait in BufferedUpdatesStream when we apply all necessary updates
    were we can continue with other work instead of waiting.
    This change also tries to apply the packages that are not currently applied
    first in order to not unnecessarily block.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr issue #503: LUCENE-8571: Don't block on FrozenBufferedUpdates#ap...

Posted by s1monw <gi...@git.apache.org>.
Github user s1monw commented on the issue:

    https://github.com/apache/lucene-solr/pull/503
  
    I recommend to look at the diff with `?w=1` -> https://github.com/apache/lucene-solr/pull/503/files?w=1


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr issue #503: LUCENE-8571: Don't block on FrozenBufferedUpdates#ap...

Posted by msokolov <gi...@git.apache.org>.
Github user msokolov commented on the issue:

    https://github.com/apache/lucene-solr/pull/503
  
    I'm curious if you were able to measure a good improvement from this? I guess a large delete concurrent with indexing lots of docs would stress it?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr issue #503: LUCENE-8571: Don't block on FrozenBufferedUpdates#ap...

Posted by s1monw <gi...@git.apache.org>.
Github user s1monw commented on the issue:

    https://github.com/apache/lucene-solr/pull/503
  
    pushed


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request #503: LUCENE-8571: Don't block on FrozenBufferedUpd...

Posted by mikemccand <gi...@git.apache.org>.
Github user mikemccand commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/503#discussion_r235350971
  
    --- Diff: lucene/core/src/java/org/apache/lucene/index/FrozenBufferedUpdates.java ---
    @@ -214,150 +216,174 @@ public FrozenBufferedUpdates(InfoStream infoStream, BufferedUpdates updates, Seg
     
       /** Translates a frozen packet of delete term/query, or doc values
        *  updates, into their actual docIDs in the index, and applies the change.  This is a heavy
    -   *  operation and is done concurrently by incoming indexing threads. */
    +   *  operation and is done concurrently by incoming indexing threads.
    +   *  This method will return immediately without blocking if another thread is currently
    +   *  applying the package. In order to ensure the packet has been applied, {@link #forceApply(IndexWriter)}
    +   *  must be called.
    +   *  */
       @SuppressWarnings("try")
    -  public synchronized void apply(IndexWriter writer) throws IOException {
    -    if (applied.getCount() == 0) {
    -      // already done
    -      return;
    +  boolean tryApply(IndexWriter writer) throws IOException {
    +    if (applyLock.tryLock()) {
    +      try {
    +        return forceApply(writer);
    +      } finally {
    +        applyLock.unlock();
    +      }
         }
    +    return false;
    +  }
     
    -    long startNS = System.nanoTime();
    +  /** Translates a frozen packet of delete term/query, or doc values
    +   *  updates, into their actual docIDs in the index, and applies the change.  This is a heavy
    +   *  operation and is done concurrently by incoming indexing threads.
    +   *  */
    +  boolean forceApply(IndexWriter writer) throws IOException {
    --- End diff --
    
    Can we make this `void`?  It always just `return true` now right?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request #503: LUCENE-8571: Don't block on FrozenBufferedUpd...

Posted by s1monw <gi...@git.apache.org>.
Github user s1monw closed the pull request at:

    https://github.com/apache/lucene-solr/pull/503


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr pull request #503: LUCENE-8571: Don't block on FrozenBufferedUpd...

Posted by jpountz <gi...@git.apache.org>.
Github user jpountz commented on a diff in the pull request:

    https://github.com/apache/lucene-solr/pull/503#discussion_r235314171
  
    --- Diff: lucene/core/src/java/org/apache/lucene/index/IndexWriter.java ---
    @@ -2607,7 +2607,9 @@ synchronized long publishFrozenUpdates(FrozenBufferedUpdates packet) {
         // Do this as an event so it applies higher in the stack when we are not holding DocumentsWriterFlushQueue.purgeLock:
         eventQueue.add(w -> {
           try {
    -        packet.apply(w);
    +        // we call tryApply here since we don't want to block if a refresh or a flush is already applying the
    +        // packet. The flush will retry this packet anyway to ensure all of the are applied
    --- End diff --
    
    s/all of the/all of them/


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr issue #503: LUCENE-8571: Don't block on FrozenBufferedUpdates#ap...

Posted by s1monw <gi...@git.apache.org>.
Github user s1monw commented on the issue:

    https://github.com/apache/lucene-solr/pull/503
  
    @msokolov I measures a pretty signficant lock contention on this lock without my change. with the change it went down significantly. Refreshes got 30% faster for that benchmark (small docs with 25% collision on the IDs and soft-deletes enabled). There was some minor indexing throughput improvements but nothing major.  


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


[GitHub] lucene-solr issue #503: LUCENE-8571: Don't block on FrozenBufferedUpdates#ap...

Posted by s1monw <gi...@git.apache.org>.
Github user s1monw commented on the issue:

    https://github.com/apache/lucene-solr/pull/503
  
    @mikemccand can you take a look


---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org