Skip to content

fix(core): 🐛 retain folders after capped deletion query - #2533

Open
newschapmj1 wants to merge 5 commits into
dadoonet:mainfrom
newschapmj1:fix/retain-folder-record-at-query-limit
Open

fix(core): 🐛 retain folders after capped deletion query#2533
newschapmj1 wants to merge 5 commits into
dadoonet:mainfrom
newschapmj1:fix/retain-folder-record-at-query-limit

Conversation

@newschapmj1

Copy link
Copy Markdown
Contributor
  • Retain folder records when deletion queries return 10,000 entries
  • Propagate incomplete deletion status to ancestor folders
  • Warn that deletion will continue on a later crawl
  • Add regression tests and documentation

- Retain folder records when deletion queries return 10,000 entries
- Propagate incomplete deletion status to ancestor folders
- Warn that deletion will continue on a later crawl
- Add regression tests and documentation
@newschapmj1
newschapmj1 requested a review from dadoonet as a code owner August 31, 2026 16:45
@mergify

mergify Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@newschapmj1

Copy link
Copy Markdown
Contributor Author

Fix for remove_deleted only reconciles the first 10,000 files in a directory + Orphan files
#2530

(It is a simple/limited 'Fix'- must rerun crawl to delete file >10K total)

@dadoonet

Copy link
Copy Markdown
Owner

Ha! That's smart. I never thought about the fact that a subsequent run would actually continue cleaning files and folders after 10000 items!
So if I read the code correctly, the PR consists of warning the user that something is not yet done. As this will be cleaned later on, should this be an info vs a warn?
And may be print in the logs the link to https://fscrawler.readthedocs.io/en/latest/admin/status.html#forcing-a-new-scan, so they would know how "not to wait" too long.

Which makes me think that we can probably test this with an integration test:

  1. Create 1 marker file, 10 000 + x random files between 1 and 500 in the test tmp dir
  2. Launch FSCrawler with update_rate = 1s on this folder
  3. wait until we have indexed 10xxx docs
  4. Pause the crawler
  5. Remove all the docs but the marker file
  6. wait until we just have one document in the index.

I'm pasting here the idea but this could be done in another PR.

@dadoonet dadoonet self-assigned this Aug 31, 2026
@dadoonet dadoonet added the bug For confirmed bugs label Aug 31, 2026
@dadoonet dadoonet added this to the 3.1 milestone Aug 31, 2026

@dadoonet dadoonet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM with only a small change to apply.
Also, could you add this as a fix in docs/source/release/3.1.md?

See an example at https://github.com/dadoonet/fscrawler/blob/main/docs/source/release/3.0.md?plain=1#L99

And please add your name at the end of the same file as it was done in https://github.com/dadoonet/fscrawler/blob/main/docs/source/release/3.0.md?plain=1#L142-L143

Thanks!

Comment thread core/src/main/java/fr/pilato/elasticsearch/crawler/fs/FsParser.java
@sonarqubecloud

sonarqubecloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

@newschapmj1

Copy link
Copy Markdown
Contributor Author

Thanks for looking at this PR so promptly.

I will submit a second PR to add a MVN test for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug For confirmed bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants