From 52a8e0c1c3dd7b339c63e1832326602e01d098bf Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Thu, 23 Jul 2026 15:19:30 -0300 Subject: [PATCH 1/4] Imporve code for java style and dead code --- .../solr/filestore/ClusterFileStore.java | 2 +- .../solr/filestore/DistribFileStore.java | 28 ++----------------- .../apache/solr/filestore/FileStoreUtils.java | 2 +- 3 files changed, 5 insertions(+), 27 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java b/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java index 80ea2066e65a..88c0108822c5 100644 --- a/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/ClusterFileStore.java @@ -400,7 +400,7 @@ private void validate(List sigs, byte[] buf) throws SolrException, IOExc throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "File store does not have any keys"); } - CryptoKeys cryptoKeys = null; + CryptoKeys cryptoKeys; try { cryptoKeys = new CryptoKeys(keys); } catch (Exception e) { diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 8cdd9a80b19a..16f0e5fb8dcb 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -39,7 +39,6 @@ import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import java.util.function.Predicate; import java.util.stream.Stream; @@ -71,7 +70,6 @@ public class DistribFileStore implements FileStore { private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private final CoreContainer coreContainer; - private Map tmpFiles = new ConcurrentHashMap<>(); private final Path solrHome; @@ -112,13 +110,6 @@ class FileInfo { this.path = path; } - ByteBuffer getFileData(boolean validate) throws IOException { - if (fileData == null) { - fileData = ByteBuffer.wrap(Files.readAllBytes(getRealPath(path))); - } - return fileData; - } - public String getMetaPath() { if (metaPath == null) { metaPath = _getMetapath(path); @@ -184,7 +175,6 @@ private boolean fetchFileFromNodeAndPersist(String fromNode) { ByteBuffer metadata; Map m; - InputStream is = null; var solrClient = coreContainer.getDefaultHttpSolrClient(); try { @@ -198,11 +188,9 @@ private boolean fetchFileFromNodeAndPersist(String fromNode) { } } catch (Exception e) { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error fetching metadata", e); - } finally { - org.apache.solr.common.util.IOUtils.closeQuietly(is); } - ByteBuffer filedata = null; + ByteBuffer filedata; try { final var fileRequest = new FileStoreApi.GetFile(path); final var fileResponse = fileRequest.processWithBaseUrl(solrClient, baseUrl, null); @@ -226,8 +214,6 @@ private boolean fetchFileFromNodeAndPersist(String fromNode) { return true; } catch (IOException ioe) { throw new SolrException(SERVER_ERROR, "Error persisting file", ioe); - } finally { - org.apache.solr.common.util.IOUtils.closeQuietly(is); } } @@ -350,7 +336,6 @@ public void put(FileEntry entry) throws IOException { private void distribute(FileInfo info) { try { - String dirName = info.path.substring(0, info.path.lastIndexOf('/')); coreContainer .getZkController() @@ -366,13 +351,11 @@ private void distribute(FileInfo info) { } catch (Exception e) { throw new SolrException(SERVER_ERROR, "Unable to create an entry in ZK", e); } - tmpFiles.put(info.path, info); List nodes = FileStoreUtils.fetchAndShuffleRemoteLiveNodes(coreContainer); int i = 0; int FETCHFROM_SRC = 50; String myNodeName = coreContainer.getZkController().getNodeName(); - String getFrom = ""; try { for (String node : nodes) { String baseUrl = @@ -387,7 +370,7 @@ private void distribute(FileInfo info) { } else { if (i == FETCHFROM_SRC) { // This is just an optimization - // at this point a bunch of nodes are already downloading from me + // at this point a bunch of nodes are already downloading from me. // I'll wait for them to finish before asking other nodes to download from each other try { Thread.sleep(2 * 1000); @@ -418,11 +401,7 @@ private void distribute(FileInfo info) { .getUpdateExecutor() .submit( () -> { - try { - Thread.sleep(10 * 1000); - } finally { - tmpFiles.remove(info.path); - } + Thread.sleep(10 * 1000); return null; }); } @@ -486,7 +465,6 @@ public void syncToAllNodes(String path) throws IOException { if (!fi.exists(true, false)) { throw new SolrException(BAD_REQUEST, "No such file : " + path); } - fi.getFileData(true); distribute(fi); } diff --git a/solr/core/src/java/org/apache/solr/filestore/FileStoreUtils.java b/solr/core/src/java/org/apache/solr/filestore/FileStoreUtils.java index b9bd841e8038..85a3a8fbd5a7 100644 --- a/solr/core/src/java/org/apache/solr/filestore/FileStoreUtils.java +++ b/solr/core/src/java/org/apache/solr/filestore/FileStoreUtils.java @@ -108,7 +108,7 @@ public static void validate( throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "Filestore does not have any public keys"); } - CryptoKeys cryptoKeys = null; + CryptoKeys cryptoKeys; try { cryptoKeys = new CryptoKeys(keys); } catch (Exception e) { From 442df0115b0d91e3d9ea7d16457702e8234cea22 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 28 Jul 2026 19:26:23 -0300 Subject: [PATCH 2/4] Remove unneeded sleep as the task in the finally step was previosly removed in this PR> --- .../solr/filestore/DistribFileStore.java | 79 ++++++++----------- 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index e2415347d07c..27ea5a1cc53c 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -357,54 +357,43 @@ private void distribute(FileInfo info) { int i = 0; int FETCHFROM_SRC = 50; String myNodeName = coreContainer.getZkController().getNodeName(); - try { - for (String node : nodes) { - String baseUrl = - coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node); - - String nodeToFetchFrom; - if (i < FETCHFROM_SRC) { - // this is to protect very large clusters from overwhelming a single node - // the first FETCHFROM_SRC nodes will be asked to fetch from this node. - // it's there in the memory now. So , it must be served fast - nodeToFetchFrom = myNodeName; - } else { - if (i == FETCHFROM_SRC) { - // This is just an optimization - // at this point a bunch of nodes are already downloading from me. - // I'll wait for them to finish before asking other nodes to download from each other - try { - Thread.sleep(2 * 1000); - } catch (Exception e) { - } + for (String node : nodes) { + String baseUrl = + coreContainer.getZkController().getZkStateReader().getBaseUrlV2ForNodeName(node); + + String nodeToFetchFrom; + if (i < FETCHFROM_SRC) { + // this is to protect very large clusters from overwhelming a single node + // the first FETCHFROM_SRC nodes will be asked to fetch from this node. + // it's there in the memory now. So , it must be served fast + nodeToFetchFrom = myNodeName; + } else { + if (i == FETCHFROM_SRC) { + // This is just an optimization + // at this point a bunch of nodes are already downloading from me. + // I'll wait for them to finish before asking other nodes to download from each other + try { + Thread.sleep(2 * 1000); + } catch (Exception e) { } - // trying to avoid the thundering herd problem when there are a very large number of - // nodes others should try to fetch it from any node where it is available. By now, - // almost FETCHFROM_SRC other nodes may have it - nodeToFetchFrom = "*"; - } - try { - final var pullFileRequest = new FileStoreApi.FetchFile(info.path); - pullFileRequest.setGetFrom(nodeToFetchFrom); - final var client = coreContainer.getDefaultHttpSolrClient(); - // fire and forget - pullFileRequest.processWithBaseUrl(client, baseUrl, null); - } catch (Exception e) { - log.info("Node: {} failed to respond for file fetch notification", node, e); - // ignore the exception - // some nodes may be down or not responding } - i++; + // trying to avoid the thundering herd problem when there are a very large number of + // nodes others should try to fetch it from any node where it is available. By now, + // almost FETCHFROM_SRC other nodes may have it + nodeToFetchFrom = "*"; } - } finally { - coreContainer - .getUpdateShardHandler() - .getUpdateExecutor() - .submit( - () -> { - Thread.sleep(10 * 1000); - return null; - }); + try { + final var pullFileRequest = new FileStoreApi.FetchFile(info.path); + pullFileRequest.setGetFrom(nodeToFetchFrom); + final var client = coreContainer.getDefaultHttpSolrClient(); + // fire and forget + pullFileRequest.processWithBaseUrl(client, baseUrl, null); + } catch (Exception e) { + log.info("Node: {} failed to respond for file fetch notification", node, e); + // ignore the exception + // some nodes may be down or not responding + } + i++; } } From 4c020754a1483e22394a9f402693b24a1014059a Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Tue, 28 Jul 2026 19:29:04 -0300 Subject: [PATCH 3/4] small comment format fix. --- .../src/java/org/apache/solr/filestore/DistribFileStore.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index 27ea5a1cc53c..fc0dd1ff133d 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -365,7 +365,7 @@ private void distribute(FileInfo info) { if (i < FETCHFROM_SRC) { // this is to protect very large clusters from overwhelming a single node // the first FETCHFROM_SRC nodes will be asked to fetch from this node. - // it's there in the memory now. So , it must be served fast + // it's there in the memory now. So, it must be served fast nodeToFetchFrom = myNodeName; } else { if (i == FETCHFROM_SRC) { From c1aac217bed859ff5517b5b80af5e0dccd45b538 Mon Sep 17 00:00:00 2001 From: Eric Pugh Date: Wed, 29 Jul 2026 12:38:14 -0400 Subject: [PATCH 4/4] Update solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java Co-authored-by: Pierre Salagnac --- .../src/java/org/apache/solr/filestore/DistribFileStore.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java index fc0dd1ff133d..d14b2d5b91ac 100644 --- a/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java +++ b/solr/core/src/java/org/apache/solr/filestore/DistribFileStore.java @@ -374,7 +374,8 @@ private void distribute(FileInfo info) { // I'll wait for them to finish before asking other nodes to download from each other try { Thread.sleep(2 * 1000); - } catch (Exception e) { + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); } } // trying to avoid the thundering herd problem when there are a very large number of