[
https://issues.apache.org/jira/browse/SOLR-13741?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16951197#comment-16951197
]
Chris M. Hostetter commented on SOLR-13741:
-------------------------------------------
+1
> possible AuditLogger bugs uncovered while hardening AuditLoggerIntegrationTest
> ------------------------------------------------------------------------------
>
> Key: SOLR-13741
> URL: https://issues.apache.org/jira/browse/SOLR-13741
> Project: Solr
> Issue Type: Bug
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
> Attachments: SOLR-13741.patch, SOLR-13741.patch, SOLR-13741.patch,
> SOLR-13741.patch, SOLR-13741.patch, SOLR-13741.patch, SOLR-13741.patch
>
>
> A while back i saw a weird non-reproducible failure from
> AuditLoggerIntegrationTest. When i started reading through that code, 2
> things jumped out at me:
> # the way the 'delay' option works is brittle, and makes assumptions about
> CPU scheduling that aren't neccessarily going to be true (and also suffers
> from the problem that Thread.sleep isn't garunteed to sleep as long as you
> ask it too)
> # the way the existing {{waitForAuditEventCallbacks(number)}} logic works by
> checking the size of a (List) {{buffer}} of recieved events in a sleep/poll
> loop, until it contains at least N items -- but the code that adds items to
> that buffer in the async Callback thread async _before_ the code that updates
> other state variables (like the global {{count}} and the patch specific
> {{resourceCounts}}) meaning that a test waiting on 3 events could "see" 3
> events added to the buffer, but calling {{assertEquals(3,
> receiver.getTotalCount())}} could subsequently fail because that variable
> hadn't been udpated yet.
> #2 was the source of the failures I was seeing, and while a quick fix for
> that specific problem would be to update all other state _before_ adding the
> event to the buffer, I set out to try and make more general improvements to
> the test:
> * eliminate the dependency on sleep loops by {{await}}-ing on concurrent data
> structures
> * harden the assertions made about the expected events recieved (updating
> some test methods that currently just assert the number of events recieved)
> * add new assertions that _only_ the expected events are recieved.
> In the process of doing this, I've found several oddities/descrepencies
> between things the test currently claims/asserts, and what *actually* happens
> under more rigerous scrutiny/assertions.
> I'll attach a patch shortly that has my (in progress) updates and inlcudes
> copious nocommits about things seem suspect. the summary of these concerns
> is:
> * SolrException status codes that do not match what the existing test says
> they should (but doesn't assert)
> * extra AuditEvents occuring that the existing test does not expect
> * AuditEvents for incorrect credentials that do not at all match the expected
> AuditEvent in the existing test -- which the current test seems to miss in
> it's assertions because it's picking up some extra events from triggered by
> previuos requests earlier in the test that just happen to also match the
> asserctions.
> ...it's not clear to me if the test logic is correct and these are "code
> bugs" or if the test is faulty.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
