[GitHub] [lucene] RS146BIJAY opened a new issue, #12228: IndexWriter should clean up unreferenced files when segment merge fails
RS146BIJAY opened a new issue, #12228: URL: https://github.com/apache/lucene/issues/12228 ### Description **Current Issue** Currently, if segment merge/force merge fails because of disk full, IndexWriter does not clean up unreferenced files created during the current segment merge. I believe this change got introduced as a part of [this patch](https://issues.apache.org/jira/browse/LUCENE-6579). Lucene considers I/O exception because of disk full as a tragedy and does not clean up unreferenced file to avoid wasting IO/CPU cycles (which can happen due to segment merge retry filling up disk again). Issue here is these unreferenced files can be huge, and it unnecessarily continues to occupy space on the node. Further, in case the disk gets 100% filled up on a node, OpenSearch mark it as unhealthy. This is one of the major reason for node drop (unhealthy nodes) for our Cx. **Solution** Lucene should delete these unreferenced files in case segment merge fails (OpenSearch cannot handle this as Lucene closes IndexWriter in case of tragic exception). To avoid wasting IO/CPU cycles, we can change the merge policy to not allow segment merge in case enough amount of space is not available (segment merge can cause disk to get full). ### Version and environment details _No response_ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] shaikhu commented on a diff in pull request #12201: Github 10633 Update Javadoc comment to mention gradle instead of ant
shaikhu commented on code in PR #12201:
URL: https://github.com/apache/lucene/pull/12201#discussion_r1161643343
##
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java:
##
@@ -147,9 +147,10 @@ public class TestBackwardsCompatibility extends
LuceneTestCase {
//
// -
//
- // To generate backcompat indexes with the current default codec, run the
following ant command:
- // ant test -Dtestcase=TestBackwardsCompatibility
-Dtests.bwcdir=/path/to/store/indexes
- // -Dtests.codec=default -Dtests.useSecurityManager=false
+ // To generate backcompat indexes with the current default codec, run the
following gradle
+ // command:
+ // gradlew test -Dtestcase=TestBackwardsCompatibility
-Dtests.bwcdir=/path/to/store/indexes
Review Comment:
> I don't think `-Dtestcase` works with gradle, please refer to this:
https://stackoverflow.com/questions/22505533/how-to-run-only-one-unit-test-class-using-gradle
>
> Also maybe give the command a try to verify your change?
Yes, you're correct it `-Dtestcase` doesn't work with Gradle. Testing
locally the following command worked
```gradlew test --tests TestBackwardsCompatibility```.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] shaikhu commented on a diff in pull request #12201: Github 10633 Update Javadoc comment to mention gradle instead of ant
shaikhu commented on code in PR #12201:
URL: https://github.com/apache/lucene/pull/12201#discussion_r1161643725
##
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java:
##
@@ -147,9 +147,10 @@ public class TestBackwardsCompatibility extends
LuceneTestCase {
//
// -
//
- // To generate backcompat indexes with the current default codec, run the
following ant command:
- // ant test -Dtestcase=TestBackwardsCompatibility
-Dtests.bwcdir=/path/to/store/indexes
- // -Dtests.codec=default -Dtests.useSecurityManager=false
+ // To generate backcompat indexes with the current default codec, run the
following gradle
+ // command:
+ // gradlew test -Dtestcase=TestBackwardsCompatibility
-Dtests.bwcdir=/path/to/store/indexes
Review Comment:
> Even better, read this help guide:
https://github.com/apache/lucene/blob/main/help/tests.txt#L51-L69
Thanks. See 886a009
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on issue #12228: IndexWriter should clean up unreferenced files when segment merge fails
rmuir commented on issue #12228: URL: https://github.com/apache/lucene/issues/12228#issuecomment-1501753122 as soon as you open a new indexwriter on the index, it will delete the unnecessary files. i don't think indexwriter should try to be a superhero and do dangerous things such as deleting files when handling exceptions. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] RS146BIJAY commented on issue #12228: IndexWriter should clean up unreferenced files when segment merge fails
RS146BIJAY commented on issue #12228: URL: https://github.com/apache/lucene/issues/12228#issuecomment-1501830603 @rmuir Thanks for response. For non tragic exceptions, IndexWriter [even now deletes unreferenced files](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java#L2500). With the above [commit](https://issues.apache.org/jira/browse/LUCENE-6579) I believe it is not deleting un-referenced files for tragedy. Also we tried to delete unreferenced files incase segment merge fails by creating a new index writer instance. Issue here is Segment merge thread inside Lucene [removes lock](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java#L2514) asynchronously after [tragedy event is set](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java#L5604) and [response is returned](https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/index/IndexWriter.java#L2137) to OpenSearch. So there was race condition scenario that when we tried to create new index writer instance it maybe possible that previous instance of index writer (on which we called force merge) has not removed the lock yet and instance creation failed due to new IndexWriter unable to obtain lock. Let me know if you have any suggestions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on issue #12228: IndexWriter should clean up unreferenced files when segment merge fails
rmuir commented on issue #12228: URL: https://github.com/apache/lucene/issues/12228#issuecomment-1501874597 on a tragedy it is unsafe to delete files or anything like that. Tragic exception means just that, time to shut down. For example it can be triggered by `VirtualMachineError`, which means it is unsafe to trust the state of the JVM, so we need to avoid doing anything destructive such as deleting files. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] RS146BIJAY commented on issue #12228: IndexWriter should clean up unreferenced files when segment merge fails due to disk full
RS146BIJAY commented on issue #12228: URL: https://github.com/apache/lucene/issues/12228#issuecomment-1501907350 I agree exceptions like `VirtualMachineError` should be considered as tragic and we should avoid deleting files in those case. Actually my major concern was why segment merge failure due to disk full is considered tragedy (updated description)? I believe IOException thrown due to disk full should not be considered as tragedy as it will cause unreferenced file (some files are huge) to continue to occupy space on the node though it is not available for query/update. Disk Full does not necessarily means there is an issue on the system/disk and operation like deleting files should be allowed. If wasting IO/CPU cycles is the only concern as mentioned in [this issue](https://issues.apache.org/jira/browse/LUCENE-6579), why not modify the merge policy itself to not allow segment merge in case enough space is not available. Let me know your thoughts. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] rmuir commented on issue #12228: IndexWriter should clean up unreferenced files when segment merge fails due to disk full
rmuir commented on issue #12228: URL: https://github.com/apache/lucene/issues/12228#issuecomment-1502025869 After reviewing the original issue again, I think the current behavior is correct, it should be tragic and requires human intervention to fix so that there is enough space. Treating it as non-fatal would just means that resources are wasted as @mikemccand describes on the original issue. Modifying the merge policy to "do nothing" if there is no disk space available is not a good solution: you can't just pretend there is no problem and continue to go on indexing without any merges happening, this will break down (not just performance but you'll run out of open file handles and hit other problems too). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] vigyasharma commented on issue #12203: Scalable merge/compaction of big doc values segments.
vigyasharma commented on issue #12203: URL: https://github.com/apache/lucene/issues/12203#issuecomment-1502180974 > My basic idea is to write each field in parallel to a separate file and then perform a low-level merge of the binary data (just appending bytes to the final file). After that, I can rewrite only the metadata to update the offsets. This is an interesting approach, I'd like to explore it more. There has been some discussion on this problem in #9626 , that you might find useful. I think it makes sense to chip away at this problem with one format type at a time. Let's start with the approach you have in mind for doc-values. If you want to raise a PR for this, I'm happy to iterate with you on it. We can start with a draft PR that demonstrates the idea first, and benchmark it for viability. And eventually refine it to consolidate across the general segment merging framework. I was playing around with doing this for the postings format some time back. It has been on the shelf for some time, but I guess it's time to dust it off and try again. Hopefully, we can collaborate on some common learnings. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] kartg opened a new pull request, #12229: Change the access modifier for the "expert" readLatestCommit API to public
kartg opened a new pull request, #12229: URL: https://github.com/apache/lucene/pull/12229 ### Description The purpose of this change is to change the access modifier for the "expert" variant of the `readLatestCommit` API from package-private to public. This change also includes a unit test for the new public API. As per the contribution guidelines, `./gradlew tidy` was run and `./gradlew check` passes. In https://github.com/apache/lucene-solr/pull/2212, a variant of the `readLatestCommit` API was added that allowed read-only access to older Lucene indices. However, the only public entrypoints to the feature relied on an `IndexCommit`. [OpenSearch](https://github.com/opensearch-project/OpenSearch) currently has an experimental feature to enable read-only, searchable access to [snapshots](https://opensearch.org/docs/latest/tuning-your-cluster/availability-and-recovery/snapshots/searchable_snapshot/), and I'd like to leverage Lucene's "expert" API to enable access to older snapshots. However, the `Engine` implementations in OpenSearch track `SegmentInfos` objects rather than `IndexCommit` so the experimental implementation currently [uses a workaround](https://github.com/opensearch-project/OpenSearch/issues/7084). Removing the workaround would help us move this feature closer to GA. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
[GitHub] [lucene] zhaih commented on a diff in pull request #12201: Github 10633 Update Javadoc comment to mention gradle instead of ant
zhaih commented on code in PR #12201:
URL: https://github.com/apache/lucene/pull/12201#discussion_r1162356564
##
lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBackwardsCompatibility.java:
##
@@ -227,7 +228,7 @@ public void testCreateMoreTermsIndex() throws Exception {
Thread.sleep(10);
}
- // ant test -Dtestcase=TestBackwardsCompatibility
-Dtestmethod=testCreateSortedIndex
+ // gradlew test -Dtestcase=TestBackwardsCompatibility
-Dtestmethod=testCreateSortedIndex
Review Comment:
Here needs to be updated as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
