[SOLR-13314] restore ability for BaseDistributedSearchTestCase subclasses to use null keys in "handle" when comparing responses

• Type: Sub-task
• Status: Open
• Priority: Major
• Resolution: Unresolved
• Assignee: Mark Miller
• Reporter: Chris M. Hostetter
• Created: 11/Mar/19 17:12
• Updated: 11/Mar/19 17:15

Description

Background from SOLR-13300 ...

Hoss:

In Nov 2018, as part of his SOLR-12801 work, Mark Miller's 75b183196798232aa6f2dcaaaab117f309119053 commit changed this test to just remove the handle.put(null, SKIP); call - evidently because he felt it was important to change BaseDistributedSearchTestCase.handle from a HashMap to a ConcurrentHashMap (which doesn't support null keys) ...

Mark:

That change ended up being annoying, it was not for the hell of it. Tests often don't respect shared base variables when they are not thread safe, timing changes can expose those problems. It is a good idea to make those base variables thread safe as a proper defense, but this skip stuff is a real problem.

Attached a patch that allows for null as a key again and also allows iteration on the map without errant concurrent modification exceptions.

I think this class is a good idea because it's difficult to allow raw access to our impl for this in ways that are fully thread safe and test friendly when accessed by subclasses. By exposing less surface area it is easier to prevent future accidental misuse and ensure thread safety.

Issue Links

• is related to: SOLR-13300 Reproducing seed in master for DistributedFacetExistsSmallTest