Skip to content

Commit b50c474

Browse files
author
Jay Deng
committed
Make IndexStoreListener a pluggable interface
1 parent 4213cc2 commit b50c474

File tree

6 files changed

+129
-34
lines changed

6 files changed

+129
-34
lines changed

server/src/main/java/org/opensearch/env/NodeEnvironment.java

+1-14
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import org.opensearch.index.IndexSettings;
7272
import org.opensearch.index.shard.ShardPath;
7373
import org.opensearch.index.store.FsDirectoryFactory;
74+
import org.opensearch.index.store.IndexStoreListener;
7475
import org.opensearch.monitor.fs.FsInfo;
7576
import org.opensearch.monitor.fs.FsProbe;
7677
import org.opensearch.monitor.jvm.JvmInfo;
@@ -1412,18 +1413,4 @@ private static void tryWriteTempFile(Path path) throws IOException {
14121413
}
14131414
}
14141415
}
1415-
1416-
/**
1417-
* A listener that is executed on per-index and per-shard store events, like deleting shard path
1418-
*
1419-
* @opensearch.internal
1420-
*/
1421-
public interface IndexStoreListener {
1422-
default void beforeShardPathDeleted(ShardId shardId, IndexSettings indexSettings, NodeEnvironment env) {}
1423-
1424-
default void beforeIndexPathDeleted(Index index, IndexSettings indexSettings, NodeEnvironment env) {}
1425-
1426-
IndexStoreListener EMPTY = new IndexStoreListener() {
1427-
};
1428-
}
14291416
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* SPDX-License-Identifier: Apache-2.0
3+
*
4+
* The OpenSearch Contributors require contributions made to
5+
* this file be licensed under the Apache-2.0 license or a
6+
* compatible open source license.
7+
*/
8+
9+
package org.opensearch.index.store;
10+
11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
13+
import org.apache.logging.log4j.message.ParameterizedMessage;
14+
import org.opensearch.common.annotation.PublicApi;
15+
import org.opensearch.core.index.Index;
16+
import org.opensearch.core.index.shard.ShardId;
17+
import org.opensearch.env.NodeEnvironment;
18+
import org.opensearch.index.IndexSettings;
19+
20+
import java.util.List;
21+
22+
/**
23+
* A listener that is executed on per-index and per-shard store events, like deleting shard path
24+
*
25+
* @opensearch.api
26+
*/
27+
@PublicApi(since = "2.19.0")
28+
public interface IndexStoreListener {
29+
default void beforeShardPathDeleted(ShardId shardId, IndexSettings indexSettings, NodeEnvironment env) {}
30+
31+
default void beforeIndexPathDeleted(Index index, IndexSettings indexSettings, NodeEnvironment env) {}
32+
33+
IndexStoreListener EMPTY = new IndexStoreListener() {
34+
};
35+
36+
/**
37+
* A Composite listener that multiplexes calls to each of the listeners methods.
38+
*/
39+
final class CompositeIndexStoreListener implements IndexStoreListener {
40+
private final List<IndexStoreListener> listeners;
41+
private final Logger logger = LogManager.getLogger(CompositeIndexStoreListener.class);
42+
43+
public CompositeIndexStoreListener(List<IndexStoreListener> listeners) {
44+
this.listeners = listeners;
45+
}
46+
47+
public void addListener(IndexStoreListener listener) {
48+
this.listeners.add(listener);
49+
}
50+
51+
@Override
52+
public void beforeShardPathDeleted(ShardId shardId, IndexSettings indexSettings, NodeEnvironment env) {
53+
for (IndexStoreListener listener : listeners) {
54+
try {
55+
listener.beforeShardPathDeleted(shardId, indexSettings, env);
56+
} catch (Exception e) {
57+
logger.warn(() -> new ParameterizedMessage("beforeShardPathDeleted listener [{}] failed", listener), e);
58+
}
59+
}
60+
}
61+
62+
@Override
63+
public void beforeIndexPathDeleted(Index index, IndexSettings indexSettings, NodeEnvironment env) {
64+
for (IndexStoreListener listener : listeners) {
65+
try {
66+
listener.beforeIndexPathDeleted(index, indexSettings, env);
67+
} catch (Exception e) {
68+
logger.warn(() -> new ParameterizedMessage("beforeIndexPathDeleted listener [{}] failed", listener), e);
69+
}
70+
}
71+
}
72+
}
73+
}

server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheCleaner.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import org.opensearch.env.NodeEnvironment;
1919
import org.opensearch.index.IndexSettings;
2020
import org.opensearch.index.shard.ShardPath;
21+
import org.opensearch.index.store.IndexStoreListener;
2122

2223
import java.io.IOException;
2324
import java.nio.file.DirectoryStream;
@@ -33,7 +34,7 @@
3334
*
3435
* @opensearch.internal
3536
*/
36-
public class FileCacheCleaner implements NodeEnvironment.IndexStoreListener {
37+
public class FileCacheCleaner implements IndexStoreListener {
3738
private static final Logger logger = LogManager.getLogger(FileCacheCleaner.class);
3839

3940
private final Provider<FileCache> fileCacheProvider;

server/src/main/java/org/opensearch/node/Node.java

+11-6
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@
157157
import org.opensearch.index.recovery.RemoteStoreRestoreService;
158158
import org.opensearch.index.remote.RemoteIndexPathUploader;
159159
import org.opensearch.index.remote.RemoteStoreStatsTrackerFactory;
160+
import org.opensearch.index.store.IndexStoreListener;
160161
import org.opensearch.index.store.RemoteSegmentStoreDirectoryFactory;
161162
import org.opensearch.index.store.remote.filecache.FileCache;
162163
import org.opensearch.index.store.remote.filecache.FileCacheCleaner;
@@ -279,7 +280,6 @@
279280
import org.opensearch.wlm.tracker.QueryGroupResourceUsageTrackerService;
280281

281282
import javax.net.ssl.SNIHostName;
282-
283283
import java.io.BufferedWriter;
284284
import java.io.Closeable;
285285
import java.io.IOException;
@@ -548,11 +548,16 @@ protected Node(
548548
*/
549549
this.environment = new Environment(settings, initialEnvironment.configDir(), Node.NODE_LOCAL_STORAGE_SETTING.get(settings));
550550
Environment.assertEquivalent(initialEnvironment, this.environment);
551-
if (DiscoveryNode.isSearchNode(settings) == false) {
552-
nodeEnvironment = new NodeEnvironment(tmpSettings, environment);
553-
} else {
554-
nodeEnvironment = new NodeEnvironment(settings, environment, new FileCacheCleaner(this::fileCache));
555-
}
551+
IndexStoreListener.CompositeIndexStoreListener compositeIndexStoreListener = new IndexStoreListener.CompositeIndexStoreListener(
552+
pluginsService.filterPlugins(IndexStorePlugin.class)
553+
.stream()
554+
.map(IndexStorePlugin::getIndexStoreListener)
555+
.filter(Optional::isPresent)
556+
.map(Optional::get)
557+
.collect(Collectors.toList())
558+
);
559+
compositeIndexStoreListener.addListener(new FileCacheCleaner(this::fileCache));
560+
nodeEnvironment = new NodeEnvironment(settings, environment, new FileCacheCleaner(this::fileCache));
556561
logger.info(
557562
"node name [{}], node ID [{}], cluster name [{}], roles {}",
558563
NODE_NAME_SETTING.get(tmpSettings),

server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java

+9
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,13 @@
3939
import org.opensearch.common.annotation.PublicApi;
4040
import org.opensearch.index.IndexSettings;
4141
import org.opensearch.index.shard.ShardPath;
42+
import org.opensearch.index.store.IndexStoreListener;
4243
import org.opensearch.indices.recovery.RecoveryState;
4344

4445
import java.io.IOException;
4546
import java.util.Collections;
4647
import java.util.Map;
48+
import java.util.Optional;
4749

4850
/**
4951
* A plugin that provides alternative directory implementations.
@@ -105,4 +107,11 @@ interface RecoveryStateFactory {
105107
default Map<String, RecoveryStateFactory> getRecoveryStateFactories() {
106108
return Collections.emptyMap();
107109
}
110+
111+
/**
112+
* The {@link IndexStoreListener}s for this plugin which are triggered upon shard/index path deletion
113+
*/
114+
default Optional<IndexStoreListener> getIndexStoreListener() {
115+
return Optional.empty();
116+
}
108117
}

server/src/test/java/org/opensearch/env/NodeEnvironmentTests.java

+33-13
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import org.opensearch.core.index.shard.ShardId;
4646
import org.opensearch.gateway.MetadataStateFormat;
4747
import org.opensearch.index.IndexSettings;
48+
import org.opensearch.index.store.IndexStoreListener;
4849
import org.opensearch.node.Node;
4950
import org.opensearch.test.IndexSettingsModule;
5051
import org.opensearch.test.NodeRoles;
@@ -65,14 +66,14 @@
6566
import java.util.concurrent.atomic.AtomicInteger;
6667
import java.util.concurrent.atomic.AtomicReference;
6768

68-
import static org.opensearch.test.NodeRoles.nonClusterManagerNode;
69-
import static org.opensearch.test.NodeRoles.nonDataNode;
7069
import static org.hamcrest.CoreMatchers.equalTo;
7170
import static org.hamcrest.Matchers.arrayWithSize;
7271
import static org.hamcrest.Matchers.containsString;
7372
import static org.hamcrest.Matchers.empty;
7473
import static org.hamcrest.Matchers.not;
7574
import static org.hamcrest.Matchers.startsWith;
75+
import static org.opensearch.test.NodeRoles.nonClusterManagerNode;
76+
import static org.opensearch.test.NodeRoles.nonDataNode;
7677

7778
@LuceneTestCase.SuppressFileSystems("ExtrasFS") // TODO: fix test to allow extras
7879
public class NodeEnvironmentTests extends OpenSearchTestCase {
@@ -360,24 +361,39 @@ protected void doRun() throws Exception {
360361
}
361362

362363
public void testIndexStoreListener() throws Exception {
363-
final AtomicInteger shardCounter = new AtomicInteger(0);
364-
final AtomicInteger indexCounter = new AtomicInteger(0);
364+
final AtomicInteger shardCounter1 = new AtomicInteger(0);
365+
final AtomicInteger shardCounter2 = new AtomicInteger(0);
366+
final AtomicInteger indexCounter1 = new AtomicInteger(0);
367+
final AtomicInteger indexCounter2 = new AtomicInteger(0);
365368
final Index index = new Index("foo", "fooUUID");
366369
final ShardId shardId = new ShardId(index, 0);
367-
final NodeEnvironment.IndexStoreListener listener = new NodeEnvironment.IndexStoreListener() {
370+
final IndexStoreListener listener1 = new IndexStoreListener() {
371+
@Override
372+
public void beforeShardPathDeleted(ShardId inShardId, IndexSettings indexSettings, NodeEnvironment env) {
373+
assertEquals(shardId, inShardId);
374+
shardCounter1.incrementAndGet();
375+
}
376+
377+
@Override
378+
public void beforeIndexPathDeleted(Index inIndex, IndexSettings indexSettings, NodeEnvironment env) {
379+
assertEquals(index, inIndex);
380+
indexCounter1.incrementAndGet();
381+
}
382+
};
383+
final IndexStoreListener listener2 = new IndexStoreListener() {
368384
@Override
369385
public void beforeShardPathDeleted(ShardId inShardId, IndexSettings indexSettings, NodeEnvironment env) {
370386
assertEquals(shardId, inShardId);
371-
shardCounter.incrementAndGet();
387+
shardCounter2.incrementAndGet();
372388
}
373389

374390
@Override
375391
public void beforeIndexPathDeleted(Index inIndex, IndexSettings indexSettings, NodeEnvironment env) {
376392
assertEquals(index, inIndex);
377-
indexCounter.incrementAndGet();
393+
indexCounter2.incrementAndGet();
378394
}
379395
};
380-
final NodeEnvironment env = newNodeEnvironment(listener);
396+
final NodeEnvironment env = newNodeEnvironment(new IndexStoreListener.CompositeIndexStoreListener(List.of(listener1, listener2)));
381397

382398
for (Path path : env.indexPaths(index)) {
383399
Files.createDirectories(path.resolve("0"));
@@ -386,26 +402,30 @@ public void beforeIndexPathDeleted(Index inIndex, IndexSettings indexSettings, N
386402
for (Path path : env.indexPaths(index)) {
387403
assertTrue(Files.exists(path.resolve("0")));
388404
}
389-
assertEquals(0, shardCounter.get());
405+
assertEquals(0, shardCounter1.get());
406+
assertEquals(0, shardCounter2.get());
390407

391408
env.deleteShardDirectorySafe(new ShardId(index, 0), idxSettings);
392409

393410
for (Path path : env.indexPaths(index)) {
394411
assertFalse(Files.exists(path.resolve("0")));
395412
}
396-
assertEquals(1, shardCounter.get());
413+
assertEquals(1, shardCounter1.get());
414+
assertEquals(1, shardCounter2.get());
397415

398416
for (Path path : env.indexPaths(index)) {
399417
assertTrue(Files.exists(path));
400418
}
401-
assertEquals(0, indexCounter.get());
419+
assertEquals(0, indexCounter1.get());
420+
assertEquals(0, indexCounter2.get());
402421

403422
env.deleteIndexDirectorySafe(index, 5000, idxSettings);
404423

405424
for (Path path : env.indexPaths(index)) {
406425
assertFalse(Files.exists(path));
407426
}
408-
assertEquals(1, indexCounter.get());
427+
assertEquals(1, indexCounter1.get());
428+
assertEquals(1, indexCounter2.get());
409429
assertTrue("LockedShards: " + env.lockedShards(), env.lockedShards().isEmpty());
410430
env.close();
411431
}
@@ -680,7 +700,7 @@ public NodeEnvironment newNodeEnvironment() throws IOException {
680700
return newNodeEnvironment(Settings.EMPTY);
681701
}
682702

683-
public NodeEnvironment newNodeEnvironment(NodeEnvironment.IndexStoreListener listener) throws IOException {
703+
public NodeEnvironment newNodeEnvironment(IndexStoreListener listener) throws IOException {
684704
Settings build = buildEnvSettings(Settings.EMPTY);
685705
return new NodeEnvironment(build, TestEnvironment.newEnvironment(build), listener);
686706
}

0 commit comments

Comments
 (0)