Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16165: Rare deadlock in SlotAcc initialization #819

Merged
merged 3 commits into from
May 2, 2022
Merged

Conversation

noblepaul
Copy link
Contributor

@noblepaul noblepaul commented Apr 25, 2022

@magibney
Copy link
Contributor

magibney commented Apr 25, 2022

Another alternative would be to just move DEV_NULL_SLOT_ACC to FacetProcessor or FacetFieldProcessor -- associating it with where it's used vs. what it is.

I thought about it. But, this is a general constant and could be used elsewhere as well

@noblepaul noblepaul merged commit b73a28c into main May 2, 2022
@risdenk risdenk deleted the jira/solr16165 branch October 20, 2022 19:36
* This CountSlotAcc exists as a /dev/null sink for callers of collect(...) and other "write"-type
* methods. It should be used in contexts where "read"-type access methods will never be called.
*/
private static class DevNullCountSlotAcc extends SlotAcc.CountSlotAcc {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too late (already merged) but I think "NoOp" would be a more meaningful name than a reference to this Unix mechanism that isn't really 1-for-1 with what we have here. If this were an OutputStream to no-where, I'd think DevNull is perfect :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks for calling this out. (extra context: the naming was merged long ago, with SOLR-13132).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this DevNull is kinda lame. Anyway , this is a private class and we can change it if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants