Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: None
-
Component/s: core/store
-
Labels:None
-
Lucene Fields:New
Description
SimpleRateLimiter.pause() uses an uncheck cast of longs to ints:
Thread.sleep((int) (pauseNS/1000000), (int) (pauseNS % 1000000));
Although we check that pauseNS is positive, however if it's large enough the cast to int produces a negative value, causing Thread.sleep to throw an exception.
We should protect for it.
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Simple patch w/ test.
Commit 1641584 from Michael McCandless in branch 'dev/trunk'
[ https://svn.apache.org/r1641584 ]
LUCENE-6075: don't overflow int in SimpleRateLimiter
Commit 1641585 from Michael McCandless in branch 'dev/branches/branch_5x'
[ https://svn.apache.org/r1641585 ]
LUCENE-6075: don't overflow int in SimpleRateLimiter
Thanks Boaz Leskes!
Thank you for fixing it.
I ran into it in code that ran in a VM and I suspect (though can't be sure) it had something to do with virtualized time. I wonder if it makes sense to have a "sanity check" upper bound to rate limiting - as sleeping for 25 days is most likely not the intended behaviour..
reopen for backport
Commit 1642666 from Michael McCandless in branch 'dev/branches/lucene_solr_4_10'
[ https://svn.apache.org/r1642666 ]
LUCENE-6075: don't overflow int in SimpleRateLimiter
I wonder if it makes sense to have a "sanity check" upper bound to rate limiting - as sleeping for 25 days is most likely not the intended behaviour..
25 days is clearly rather silly ... but I'm not sure we should add such checks at this low level. Where would you draw the line?
Yeah, that's the tricky part which made me wonder (as opposed to a suggestion 
) The only thought I had is maintaining a last update time stamp (on top on lastNS) and if that goes backwards, adapt for it? I'm not sure how often this happens in practice though. From the reaction, I judge that it's not really often and we just have stepped into a very rare case. I'm OK with leaving as is. Thx for the fix!
Bulk close after 5.0 release.
+1, I'll fix this.