Skip to content

SOLR-16866: Reorder nested directory deletions to avoid NoSuchFileException#2349

Merged
magibney merged 6 commits intoapache:mainfrom
cowpaths:michaelgibney/SOLR-16866-deferred-directory-removal
Apr 19, 2024
Merged

SOLR-16866: Reorder nested directory deletions to avoid NoSuchFileException#2349
magibney merged 6 commits intoapache:mainfrom
cowpaths:michaelgibney/SOLR-16866-deferred-directory-removal

Conversation

@magibney
Copy link
Copy Markdown
Contributor

public boolean closeCacheValueCalled = false;
public boolean doneWithDir = false;
private boolean deleteAfterCoreClose = false;
public Set<CacheValue> removeEntries = new HashSet<>();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's nice to see one less data structure to track

Comment on lines +303 to +305
CacheValue[] ret = vals.toArray(new CacheValue[0]);
Arrays.sort(ret, (a, b) -> b.path.compareTo(a.path));
return Arrays.asList(ret);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: this would be nicer as a single statement using Java streams

Copy link
Copy Markdown
Contributor

@HoustonPutman HoustonPutman left a comment

Choose a reason for hiding this comment

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

I have to say this is confusing stuff, but it certainly looks like an improvement


@Test
public void reorderingTest() throws Exception {
// failure: 58F920FAAE5904EF
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you still seeing a failure with this seed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah, good catch. I think that crept in during development, and I neglected to remove it once the underlying issue was fixed. Definitely not failing for that seed anymore; removed that comment with dbc84ce

@magibney
Copy link
Copy Markdown
Contributor Author

Assuming we want to wait until after 9.6 for this? It should be safe, but it's really only about reducing log noise, so is pretty low priority.

@HoustonPutman
Copy link
Copy Markdown
Contributor

I've seen this cause failures when moving replicas around, so I would love to see it fixed in 9.6

@magibney magibney merged commit 72793f5 into apache:main Apr 19, 2024
magibney added a commit that referenced this pull request Apr 19, 2024
…ous NoSuchFileException (#2349)

(cherry picked from commit 72793f5)
magibney added a commit that referenced this pull request Apr 19, 2024
…ous NoSuchFileException (#2349)

(cherry picked from commit 72793f5)
tboeghk pushed a commit to otto-de/solr that referenced this pull request Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants