Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.8.1, 4.9, 6.0
    • Component/s: None
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      I was playing with merge throttling and discovered that our
      SimpleRateLimiter is throttling far more than requested; e.g. I asked
      for 50 MB/sec merge throttling, but it throttled at more like 8
      MB/sec.

      The problem is we are calling Thread.sleep on too-small (< 1 msec)
      times; on ordinary (non-real-time) JVMs, anything less than 1 msec is
      rounded up to 1 msec. Also, System.nanoTime() is somewhat costly.

      To fix this, I think we should aggregate the incoming byte count,
      until it crosses a threshold of enough bytes to warrant pausing.

      1. LUCENE-5641.patch
        9 kB
        Michael McCandless
      2. LUCENE-5641.patch
        8 kB
        Michael McCandless

        Activity

        Hide
        Michael McCandless added a comment -

        Here's a simple patch: it just pushes responsibility of adding up
        pending (thread-private) bytes to the caller, i.e. caller should not
        call RateLimiter.pause until it has written more than 5 msec worth of
        bytes.

        I played with another approach of doing aggregation inside
        RateLimiter.pause using an AtomicLong, but it quickly gets hairy, and
        at least the solution I iterated too was not scheduler-friendly (it
        would pause all threads once the IO rate was exceeded), vs this patch
        which will "take turns" pausing the threads.

        Show
        Michael McCandless added a comment - Here's a simple patch: it just pushes responsibility of adding up pending (thread-private) bytes to the caller, i.e. caller should not call RateLimiter.pause until it has written more than 5 msec worth of bytes. I played with another approach of doing aggregation inside RateLimiter.pause using an AtomicLong, but it quickly gets hairy, and at least the solution I iterated too was not scheduler-friendly (it would pause all threads once the IO rate was exceeded), vs this patch which will "take turns" pausing the threads.
        Hide
        Michael McCandless added a comment -

        New patch, removing the nocommit, I think it's ready; I'll commit soon.

        Show
        Michael McCandless added a comment - New patch, removing the nocommit, I think it's ready; I'll commit soon.
        Hide
        Robert Muir added a comment -

        Looks good, i am just a little worried about the test. For example TimeLimitingCollector's test doesnt even fail but just prints. I dont think that should block fixing this, but its just something to think about.

        Maybe later we can add unit tests (like some kind of MockRateLimiter) to test the actual logic without depending on wall-clock time?

        Show
        Robert Muir added a comment - Looks good, i am just a little worried about the test. For example TimeLimitingCollector's test doesnt even fail but just prints. I dont think that should block fixing this, but its just something to think about. Maybe later we can add unit tests (like some kind of MockRateLimiter) to test the actual logic without depending on wall-clock time?
        Hide
        Michael McCandless added a comment -

        Yeah I agree it'd be better to have a mock tester here somehow ... the test I added does assert but I'm worried about false trips. I'll commit this for now and monitor it and think about how we could better test it ...

        Show
        Michael McCandless added a comment - Yeah I agree it'd be better to have a mock tester here somehow ... the test I added does assert but I'm worried about false trips. I'll commit this for now and monitor it and think about how we could better test it ...
        Hide
        ASF subversion and git services added a comment -

        Commit 1592606 from Michael McCandless in branch 'dev/trunk'
        [ https://svn.apache.org/r1592606 ]

        LUCENE-5641: add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter

        Show
        ASF subversion and git services added a comment - Commit 1592606 from Michael McCandless in branch 'dev/trunk' [ https://svn.apache.org/r1592606 ] LUCENE-5641 : add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter
        Hide
        ASF subversion and git services added a comment -

        Commit 1592607 from Michael McCandless in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1592607 ]

        LUCENE-5641: add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter

        Show
        ASF subversion and git services added a comment - Commit 1592607 from Michael McCandless in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1592607 ] LUCENE-5641 : add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter
        Hide
        ASF subversion and git services added a comment -

        Commit 1592608 from Michael McCandless in branch 'dev/branches/lucene_solr_4_8'
        [ https://svn.apache.org/r1592608 ]

        LUCENE-5641: add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter

        Show
        ASF subversion and git services added a comment - Commit 1592608 from Michael McCandless in branch 'dev/branches/lucene_solr_4_8' [ https://svn.apache.org/r1592608 ] LUCENE-5641 : add RateLimiter.getMinPauseCheckBytes for callers to know minimal number of bytes before consulting rate limiter

          People

          • Assignee:
            Michael McCandless
            Reporter:
            Michael McCandless
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development