SOLR-13608: Sync on TrackingBackupRepo NL changes#59
Merged
gerlowskija merged 1 commit intoapache:mainfrom Apr 6, 2021
Conversation
BackupRepositoryFactory has a special case in its 'newInstance' method. For 'TrackingBackupRepository' instances, BRF will modify a NamedList representing the TBF config - adding references to the SolrResourceLoader and to BRF itself. In isolation this is innocuous, but becomes a problem when BRF.newInstance is called concurrently from different threads. Threads can race to set these NamedList keys, corrupting the NamedList in the process. (i.e. NamedList values ended up in 'key' indices, and vice versa) This commit adds synchronization to this NL initialization, to ensure that the NL values are only set by a single thread.
gerlowskija
deleted the
SOLR_13608_synchronize_modification_of_trackingrepo_config
branch
epugh
pushed a commit
to epugh/solr
that referenced
this pull request
Oct 22, 2021
BackupRepositoryFactory has a special case in its 'newInstance' method. For 'TrackingBackupRepository' instances, BRF will modify a NamedList representing the TBF config - adding references to the SolrResourceLoader and to BRF itself. In isolation this is innocuous, but becomes a problem when BRF.newInstance is called concurrently from different threads. Threads can race to set these NamedList keys, corrupting the NamedList in the process. (i.e. NamedList values ended up in 'key' indices, and vice versa) This commit adds synchronization to this NL initialization, to ensure that the NL values are only set by a single thread.
chatman
pushed a commit
to fullstorydev/solr
that referenced
this pull request
Jun 13, 2022
* port changes fromnoble/prometheus-stats
* Read CoreContainer directly from HttpServletRequest
Co-authored-by: Noble Paul <noble@apache.org>
hiteshk25
referenced
this pull request
in cowpaths/fullstory-solr
Oct 14, 2022
* port changes fromnoble/prometheus-stats * Read CoreContainer directly from HttpServletRequest Co-authored-by: Ishan Chattopadhyaya <ishan@apache.org>
hiteshk25
referenced
this pull request
in cowpaths/fullstory-solr
Nov 1, 2022
* port changes fromnoble/prometheus-stats * Read CoreContainer directly from HttpServletRequest Co-authored-by: Ishan Chattopadhyaya <ishan@apache.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
BackupRepositoryFactory has a special case in its 'newInstance' method.
For 'TrackingBackupRepository' instances, BRF will modify a NamedList
representing the TBF config - adding references to the
SolrResourceLoader and to BRF itself.
In isolation this is innocuous, but becomes a problem when
BRF.newInstance is called concurrently from different threads. Threads
can race to set these NamedList keys, corrupting the NamedList in the
process. (i.e. NamedList values ended up in 'key' indices, and vice
versa)
Solution
This commit adds synchronization to this NL initialization, to ensure
that the NL values are only set by a single thread.
Tests
This PR itself is a fix for LocalFSCloudIncrementalBackupTest. It can be validated by beasting that test class to ensure the flakiness has been resolved.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.