From e41902652bdaf5458f2dc406dadf3f8a1f3797dd Mon Sep 17 00:00:00 2001 From: Balmukund Trivedi Date: Tue, 11 Aug 2026 14:32:26 -0700 Subject: [PATCH] Derive the Elasticsearch index name of a store in one place (#4929) Every read and write path mapped a JanusGraph store name to an Elasticsearch index name through generateIndexStoreName, which lowercases the store. RestElasticSearchClient.clearStore composed the same name by hand and did not lowercase it. A mixed index stores its JanusGraph index name as the store name verbatim, and JanusGraph index names are case sensitive. So for an index named vertexByName, documents were written to and queried from _vertexbyname, while DISCARD_INDEX asked Elasticsearch to delete _vertexByName. An Elasticsearch index name is always lowercase, so that name cannot exist. Either the existence check returned false and the call did nothing, or Elasticsearch rejected the name and ManagementSystem reported that the backend does not support index removal. The documents stayed, and the schema was marked DISCARDED, so JanusGraph believed the data was gone. Remove the second derivation instead of correcting it. ElasticSearchIndex now passes the name it already derived, so ElasticSearchClient.clearStore takes the Elasticsearch index name rather than the pair it used to rebuild. Signed-off-by: Balmukund Trivedi Co-Authored-By: Claude Opus 5 (1M context) --- .../diskstorage/es/ElasticSearchClient.java | 4 +- .../diskstorage/es/ElasticSearchIndex.java | 7 +- .../es/rest/RestElasticSearchClient.java | 7 +- .../es/rest/RestClientClearStoreTest.java | 118 ++++++++++++++++++ 4 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/rest/RestClientClearStoreTest.java diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java index cf4cf60159..c7ab637b82 100644 --- a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java +++ b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchClient.java @@ -52,7 +52,9 @@ public interface ElasticSearchClient extends Closeable { void deleteIndex(String indexName) throws IOException; - void clearStore(String indexName, String storeName) throws IOException; + //Takes the Elasticsearch index name of the store, already derived by the caller, so that the mapping from a + //JanusGraph store name to an Elasticsearch index name exists in exactly one place + void clearStore(String indexStoreName) throws IOException; void bulkRequest(List requests, String ingestPipeline) throws IOException; diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java index 4e3e5a0ff8..adbe1c6d26 100644 --- a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java +++ b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/ElasticSearchIndex.java @@ -1482,10 +1482,13 @@ public void clearStorage() throws BackendException { @Override public void clearStore(String storeName) throws BackendException { + //Derive the name the same way every read and write path does, so that a store name which is not already + //lowercase still resolves to the Elasticsearch index which actually holds its documents + final String indexStoreName = getIndexStoreName(storeName); try { - client.clearStore(indexName, storeName); + client.clearStore(indexStoreName); } catch (final Exception e) { - throw new PermanentBackendException("Could not clear store " + indexName + "_" + storeName, e); + throw new PermanentBackendException("Could not clear store " + indexStoreName, e); } } diff --git a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java index f399863065..16f08faa29 100644 --- a/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java +++ b/janusgraph-es/src/main/java/org/janusgraph/diskstorage/es/rest/RestElasticSearchClient.java @@ -387,10 +387,9 @@ public void deleteIndex(String indexName) throws IOException { } @Override - public void clearStore(String indexName, String storeName) throws IOException { - String name = indexName + "_" + storeName; - if (indexExists(name)) { - performRequest(REQUEST_TYPE_DELETE, REQUEST_SEPARATOR + indexName + "_" + storeName, null); + public void clearStore(String indexStoreName) throws IOException { + if (indexExists(indexStoreName)) { + performRequest(REQUEST_TYPE_DELETE, REQUEST_SEPARATOR + indexStoreName, null); } } diff --git a/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/rest/RestClientClearStoreTest.java b/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/rest/RestClientClearStoreTest.java new file mode 100644 index 0000000000..abcc496318 --- /dev/null +++ b/janusgraph-es/src/test/java/org/janusgraph/diskstorage/es/rest/RestClientClearStoreTest.java @@ -0,0 +1,118 @@ +// Copyright 2026 JanusGraph Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package org.janusgraph.diskstorage.es.rest; + +import org.apache.http.StatusLine; +import org.elasticsearch.client.Request; +import org.elasticsearch.client.Response; +import org.elasticsearch.client.RestClient; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +//clearStore deletes the Elasticsearch index which backs a store, and is reached through +//SchemaAction.DISCARD_INDEX. It must use the index name it is given verbatim, because the caller has already mapped +//the JanusGraph store name to it. Deriving the name a second time here is what let a store name containing an +//uppercase character target an index which cannot exist, since Elasticsearch index names are always lowercase. +@ExtendWith(MockitoExtension.class) +public class RestClientClearStoreTest { + + private static final String INDEX_STORE_NAME = "janusgraph_vertexbyname"; + + @Mock + private RestClient restClientMock; + + @Mock + private StatusLine statusLine; + + @Captor + private ArgumentCaptor requestCaptor; + + private RestElasticSearchClient createClient() throws IOException { + //The version lookup during instantiation is not under test + when(restClientMock.performRequest(any())).thenThrow(new IOException()); + final RestElasticSearchClient clientUnderTest = new RestElasticSearchClient(restClientMock, 0, false, + 0, Collections.emptySet(), 0, 0, 100_000_000); + Mockito.reset(restClientMock); + return clientUnderTest; + } + + private Response response(int statusCode) { + when(statusLine.getStatusCode()).thenReturn(statusCode); + final Response response = Mockito.mock(Response.class); + when(response.getStatusLine()).thenReturn(statusLine); + return response; + } + + @Test + public void shouldDeleteTheIndexItIsGiven() throws IOException { + try (RestElasticSearchClient clientUnderTest = createClient()) { + final Response found = response(200); + when(restClientMock.performRequest(any())).thenReturn(found); + clientUnderTest.clearStore(INDEX_STORE_NAME); + + //The existence check, then the deletion, both against the given name + verify(restClientMock, Mockito.times(2)).performRequest(requestCaptor.capture()); + final List requests = requestCaptor.getAllValues(); + assertEquals("HEAD", requests.get(0).getMethod()); + assertEquals("/" + INDEX_STORE_NAME, requests.get(0).getEndpoint()); + assertEquals("DELETE", requests.get(1).getMethod()); + assertEquals("/" + INDEX_STORE_NAME, requests.get(1).getEndpoint()); + } + } + + @Test + public void shouldNotDeleteAnIndexWhichDoesNotExist() throws IOException { + try (RestElasticSearchClient clientUnderTest = createClient()) { + final Response notFound = response(404); + when(restClientMock.performRequest(any())).thenReturn(notFound); + clientUnderTest.clearStore(INDEX_STORE_NAME); + + //Only the existence check is issued, so no deletion followed it + verify(restClientMock, Mockito.times(1)).performRequest(requestCaptor.capture()); + assertEquals("HEAD", requestCaptor.getValue().getMethod()); + } + } + + @Test + public void shouldNotLowercaseTheNameItIsGiven() throws IOException { + //Deriving the name is the caller's job. If this method lowercased as well, a store whose Elasticsearch index + //genuinely differs in case could not be addressed at all + final String mixedCase = "Janusgraph_MixedCase"; + try (RestElasticSearchClient clientUnderTest = createClient()) { + final Response found = response(200); + when(restClientMock.performRequest(any())).thenReturn(found); + clientUnderTest.clearStore(mixedCase); + + verify(restClientMock, Mockito.times(2)).performRequest(requestCaptor.capture()); + assertTrue(requestCaptor.getAllValues().stream() + .allMatch(request -> request.getEndpoint().equals("/" + mixedCase))); + } + } +}