From 1e582dee74e69c5a3f2870f4ec5844755b274ca9 Mon Sep 17 00:00:00 2001 From: Luke Kot-Zaniewski Date: Sat, 21 Feb 2026 16:36:51 -0500 Subject: [PATCH 1/3] Make LukeRequestHandler::getFirstLiveDoc get the first live doc --- .../handler/admin/LukeRequestHandler.java | 4 +-- .../handler/admin/LukeRequestHandlerTest.java | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/LukeRequestHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/LukeRequestHandler.java index e3d07b1cbca7..b5879024d100 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/LukeRequestHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/LukeRequestHandler.java @@ -472,7 +472,7 @@ private static Document getFirstLiveDoc(Terms terms, LeafReader reader) throws I StoredFields storedFields = reader.storedFields(); // Deal with the chance that the first bunch of terms are in deleted documents. Is there a // better way? - for (int idx = 0; idx < 1000 && postingsEnum == null; ++idx) { + for (int idx = 0; idx < 1000; ++idx) { text = termsEnum.next(); // Ran off the end of the terms enum without finding any live docs with that field in them. if (text == null) { @@ -481,7 +481,7 @@ private static Document getFirstLiveDoc(Terms terms, LeafReader reader) throws I postingsEnum = termsEnum.postings(postingsEnum, PostingsEnum.NONE); final Bits liveDocs = reader.getLiveDocs(); if (postingsEnum.nextDoc() != DocIdSetIterator.NO_MORE_DOCS) { - if (liveDocs != null && liveDocs.get(postingsEnum.docID())) { + if (liveDocs != null && !liveDocs.get(postingsEnum.docID())) { continue; } return storedFields.document(postingsEnum.docID()); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java index 02d7066d215b..c033960592d8 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java @@ -20,6 +20,7 @@ import java.util.EnumSet; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.luke.FieldFlag; +import org.apache.solr.index.NoMergePolicyFactory; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.schema.CustomAnalyzerStrField; import org.apache.solr.schema.IndexSchema; @@ -294,4 +295,39 @@ public void testCatchAllCopyField() throws Exception { deleteCore(); initCore("solrconfig.xml", "schema12.xml"); } + + /** + * Tests that Luke reports index flags for fields even when a document has been deleted. Uses + * NoMergePolicy so the deleted doc remains in the same segment, keeping liveDocs non-null and + * exposing the inverted liveDocs check in getFirstLiveDoc. + */ + @Test + public void testIndexFlagsWithDeletedDocs() throws Exception { + deleteCore(); + systemSetPropertySolrTestsMergePolicyFactory(NoMergePolicyFactory.class.getName()); + try { + initCore("solrconfig.xml", "schema12.xml"); + + // Three docs in a single commit so they share one segment. Delete the lexical edges + // ("aaa" and "ccc") and keep the middle term ("bbb") live, so the test is robust + // regardless of whether terms are iterated in ascending or descending order. + assertU(adoc("id", "1", "solr_s", "aaa")); + assertU(adoc("id", "2", "solr_s", "bbb")); + assertU(adoc("id", "3", "solr_s", "ccc")); + assertU(commit()); + + assertU(delI("1")); + assertU(delI("3")); + assertU(commit()); + + assertQ( + "index flags should be present for solr_s despite deletion in segment", + req("qt", "/admin/luke", "fl", "solr_s"), + getFieldXPathPrefix("solr_s") + "[@name='index']"); + } finally { + deleteCore(); + System.clearProperty(SYSTEM_PROPERTY_SOLR_TESTS_MERGEPOLICYFACTORY); + initCore("solrconfig.xml", "schema12.xml"); + } + } } From 3f6908cc0862cc1a5dcf622e88288a950d9dec3a Mon Sep 17 00:00:00 2001 From: Luke Kot-Zaniewski Date: Sat, 21 Feb 2026 16:50:13 -0500 Subject: [PATCH 2/3] make test more obvious --- .../handler/admin/LukeRequestHandlerTest.java | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java index c033960592d8..f3f8852926f1 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java @@ -308,22 +308,42 @@ public void testIndexFlagsWithDeletedDocs() throws Exception { try { initCore("solrconfig.xml", "schema12.xml"); - // Three docs in a single commit so they share one segment. Delete the lexical edges - // ("aaa" and "ccc") and keep the middle term ("bbb") live, so the test is robust - // regardless of whether terms are iterated in ascending or descending order. + // Three docs in a single commit so they share one segment. Delete the middle term + // ("bbb") and keep the lexical edges ("aaa" and "ccc") live, so the first term + // encountered is always a live doc regardless of ascending or descending iteration. + // The bug's inverted liveDocs check skips live docs, so getFirstLiveDoc returns null + // and index flags go missing. assertU(adoc("id", "1", "solr_s", "aaa")); assertU(adoc("id", "2", "solr_s", "bbb")); assertU(adoc("id", "3", "solr_s", "ccc")); assertU(commit()); - assertU(delI("1")); - assertU(delI("3")); + assertU(delI("2")); assertU(commit()); assertQ( "index flags should be present for solr_s despite deletion in segment", req("qt", "/admin/luke", "fl", "solr_s"), getFieldXPathPrefix("solr_s") + "[@name='index']"); + + // Now test the inverse: delete the edges and keep the middle. The first term + // encountered ("aaa" or "ccc") is deleted, so getFirstLiveDoc must skip it and + // continue to the live term ("bbb"). This exercises the retry loop — without it, + // the method gives up after the first deleted term. + clearIndex(); + assertU(adoc("id", "4", "solr_s", "aaa")); + assertU(adoc("id", "5", "solr_s", "bbb")); + assertU(adoc("id", "6", "solr_s", "ccc")); + assertU(commit()); + + assertU(delI("4")); + assertU(delI("6")); + assertU(commit()); + + assertQ( + "index flags should be present for solr_s when edges are deleted", + req("qt", "/admin/luke", "fl", "solr_s"), + getFieldXPathPrefix("solr_s") + "[@name='index']"); } finally { deleteCore(); System.clearProperty(SYSTEM_PROPERTY_SOLR_TESTS_MERGEPOLICYFACTORY); From bb33c1f6bdfe81ba1aa934eb832bd88ffe6f22ee Mon Sep 17 00:00:00 2001 From: Luke Kot-Zaniewski Date: Sat, 21 Feb 2026 21:34:35 -0500 Subject: [PATCH 3/3] mention jira in test --- .../apache/solr/handler/admin/LukeRequestHandlerTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java index f3f8852926f1..8edb23d11a8b 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/LukeRequestHandlerTest.java @@ -297,9 +297,9 @@ public void testCatchAllCopyField() throws Exception { } /** - * Tests that Luke reports index flags for fields even when a document has been deleted. Uses - * NoMergePolicy so the deleted doc remains in the same segment, keeping liveDocs non-null and - * exposing the inverted liveDocs check in getFirstLiveDoc. + * SOLR-18125: Tests that Luke reports index flags for fields even when a document has been + * deleted. Uses NoMergePolicy so the deleted doc remains in the same segment, keeping liveDocs + * non-null and exposing the inverted liveDocs check in getFirstLiveDoc. */ @Test public void testIndexFlagsWithDeletedDocs() throws Exception {