Skip to content

Commit 49cbe0f

Browse files
committed
More misc fixes
Signed-off-by: Atri Sharma <atri.jiit@gmail.com>
2 parents 6d822bf + 4e877c3 commit 49cbe0f

File tree

87 files changed

+3213
-1547
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

87 files changed

+3213
-1547
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,20 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
2323
- Update to `almalinux:10` ([#20482](https://github.com/opensearch-project/OpenSearch/pull/20482))
2424
- Add X-Request-Id to uniquely identify a search request ([#19798](https://github.com/opensearch-project/OpenSearch/pull/19798))
2525
- Added TopN selection logic for streaming terms aggregations ([#20481](https://github.com/opensearch-project/OpenSearch/pull/20481))
26+
- Added support for Intra Segment Search ([#19704](https://github.com/opensearch-project/OpenSearch/pull/19704))
27+
- Introduce AdditionalCodecs and EnginePlugin::getAdditionalCodecs hook to allow additional Codec registration ([#20411](https://github.com/opensearch-project/OpenSearch/pull/20411))
2628

2729
### Changed
2830
- Handle custom metadata files in subdirectory-store ([#20157](https://github.com/opensearch-project/OpenSearch/pull/20157))
2931
- Add support for missing proto fields in GRPC FunctionScore and Highlight ([#20169](https://github.com/opensearch-project/OpenSearch/pull/20169))
3032
- Ensure all modules are included in INTEG_TEST testcluster distribution ([#20241](https://github.com/opensearch-project/OpenSearch/pull/20241))
3133
- Cleanup HttpServerTransport.Dispatcher in Netty tests ([#20160](https://github.com/opensearch-project/OpenSearch/pull/20160))
34+
- Use compact object headers with JDK25+ ([#20392](https://github.com/opensearch-project/OpenSearch/pull/20392))
3235
- Add `cluster.initial_cluster_manager_nodes` to testClusters OVERRIDABLE_SETTINGS ([#20348](https://github.com/opensearch-project/OpenSearch/pull/20348))
3336
- Add BigInteger support for unsigned_long fields in gRPC transport ([#20346](https://github.com/opensearch-project/OpenSearch/pull/20346))
3437
- Install demo security information when running ./gradlew run -PinstalledPlugins="['opensearch-security']" ([#20372](https://github.com/opensearch-project/OpenSearch/pull/20372))
3538
- Add `Alt-Svc` header support to advertise HTTP/3 availability ([#20434](https://github.com/opensearch-project/OpenSearch/pull/20434))
39+
- Refactor streaming agg query phase planning ([#20471](https://github.com/opensearch-project/OpenSearch/pull/20471))
3640

3741
### Fixed
3842
- Fix Snapshot rename replacement unbounded length rename ([#20464](https://github.com/opensearch-project/OpenSearch/issues/20464))
@@ -51,6 +55,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
5155
- LeafReader should not remove SubReaderWrappers incase IndexWriter encounters a non aborting Exception ([#20193](https://github.com/opensearch-project/OpenSearch/pull/20193))
5256
- Fix Netty deprecation warnings in transport-reactor-netty4 module ([20429](https://github.com/opensearch-project/OpenSearch/pull/20429))
5357
- Fix stats aggregation returning zero results with `size:0`. ([20427](https://github.com/opensearch-project/OpenSearch/pull/20427))
58+
- Fix the node local term and version being truncated in logs when host providers return very long IP or host strings ([20432](https://github.com/opensearch-project/OpenSearch/pull/20432))
5459
- Remove child level directory on refresh for CompositeIndexWriter ([#20326](https://github.com/opensearch-project/OpenSearch/pull/20326))
5560
- Fixes and refactoring in stream transport to make it more robust ([#20359](https://github.com/opensearch-project/OpenSearch/pull/20359))
5661

distribution/src/config/jvm.options

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,5 @@ ${error.file}
9191
# prevent issues like "leaked" maps or performance degradation. A value of 1 effectively
9292
# disables the shared Arena pooling and uses a confined Arena for each MMapDirectory
9393
-Dorg.apache.lucene.store.MMapDirectory.sharedArenaMaxPermits=1
94+
95+
25-:-XX:+UseCompactObjectHeaders

modules/transport-grpc/spi/src/main/java/org/opensearch/transport/grpc/spi/GrpcInterceptorProvider.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
package org.opensearch.transport.grpc.spi;
99

10+
import org.opensearch.common.settings.Settings;
1011
import org.opensearch.common.util.concurrent.ThreadContext;
1112

1213
import java.util.List;
@@ -20,6 +21,12 @@
2021
*/
2122
public interface GrpcInterceptorProvider {
2223

24+
/**
25+
* Provide visibility into node settings.
26+
* @param settings for use in interceptors.
27+
*/
28+
default void initNodeSettings(Settings settings) {}
29+
2330
/**
2431
* Returns a list of ordered gRPC interceptors with access to ThreadContext.
2532
* Each interceptor must have a unique order value.

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/GrpcPlugin.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,7 @@ public Collection<Object> createComponents(
349349
// Then add plugin-provided interceptors
350350
if (!interceptorProviders.isEmpty()) {
351351
for (GrpcInterceptorProvider provider : interceptorProviders) {
352+
provider.initNodeSettings(environment.settings());
352353
orderedList.addAll(provider.getOrderedGrpcInterceptors(threadPool.getThreadContext()));
353354
}
354355

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/GrpcPluginTests.java

Lines changed: 127 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.opensearch.common.settings.Settings;
1515
import org.opensearch.common.util.concurrent.ThreadContext;
1616
import org.opensearch.core.indices.breaker.CircuitBreakerService;
17+
import org.opensearch.env.Environment;
1718
import org.opensearch.plugins.ExtensiblePlugin;
1819
import org.opensearch.plugins.SecureAuxTransportSettingsProvider;
1920
import org.opensearch.protobufs.QueryContainer;
@@ -83,6 +84,9 @@ public class GrpcPluginTests extends OpenSearchTestCase {
8384
@Mock
8485
private Client client;
8586

87+
@Mock
88+
private Environment environment;
89+
8690
private NetworkService networkService;
8791

8892
private ClusterSettings clusterSettings;
@@ -102,8 +106,9 @@ public void setup() {
102106
// Create a real ClusterSettings instance with the plugin's settings
103107
plugin = new GrpcPlugin();
104108

105-
// Mock ThreadPool and ThreadContext
109+
// Mock ThreadPool/ThreadContext/Environment
106110
when(threadPool.getThreadContext()).thenReturn(new ThreadContext(Settings.EMPTY));
111+
when(environment.settings()).thenReturn(Settings.EMPTY);
107112

108113
// Set the client in the plugin
109114
plugin.createComponents(
@@ -113,7 +118,7 @@ public void setup() {
113118
null, // ResourceWatcherService
114119
null, // ScriptService
115120
null, // NamedXContentRegistry
116-
null, // Environment
121+
environment, // Environment
117122
null, // NodeEnvironment
118123
null, // NamedWriteableRegistry
119124
null, // IndexNameExpressionResolver
@@ -448,12 +453,11 @@ public void testLoadExtensionsWithDuplicateGrpcInterceptorOrder() {
448453

449454
IllegalArgumentException exception = expectThrows(
450455
IllegalArgumentException.class,
451-
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, null, null, null, null, null)
456+
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, environment, null, null, null, null)
452457
);
453458

454459
String errorMessage = exception.getMessage();
455460
assertTrue(errorMessage.contains("Multiple gRPC interceptors have the same order value [1]"));
456-
assertTrue(errorMessage.contains("ServerInterceptor")); // Mock class name will contain this
457461
assertTrue(errorMessage.contains("Each interceptor must have a unique order value"));
458462
}
459463

@@ -470,12 +474,11 @@ public void testLoadExtensionsWithMultipleProvidersAndDuplicateOrder() {
470474

471475
IllegalArgumentException exception = expectThrows(
472476
IllegalArgumentException.class,
473-
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, null, null, null, null, null)
477+
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, environment, null, null, null, null)
474478
);
475479

476480
String errorMessage = exception.getMessage();
477481
assertTrue(errorMessage.contains("Multiple gRPC interceptors have the same order value [5]"));
478-
assertTrue(errorMessage.contains("ServerInterceptor"));
479482
assertTrue(errorMessage.contains("Each interceptor must have a unique order value"));
480483
}
481484

@@ -498,12 +501,11 @@ public void testLoadExtensionsWithSameExplicitOrderInterceptors() {
498501

499502
IllegalArgumentException exception = expectThrows(
500503
IllegalArgumentException.class,
501-
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, null, null, null, null, null)
504+
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, environment, null, null, null, null)
502505
);
503506

504507
String errorMessage = exception.getMessage();
505508
assertTrue(errorMessage.contains("Multiple gRPC interceptors have the same order value [5]"));
506-
assertTrue(errorMessage.contains("ServerInterceptor"));
507509
assertTrue(errorMessage.contains("Each interceptor must have a unique order value"));
508510
}
509511

@@ -742,13 +744,31 @@ private void testRequestProcessingWithException(
742744
}
743745

744746
/**
745-
* Creates a mock interceptor with given order
747+
* Creates a no-op interceptor with the specified order.
746748
*/
747749
private OrderedGrpcInterceptor createMockInterceptor(int order) {
748-
OrderedGrpcInterceptor mock = Mockito.mock(OrderedGrpcInterceptor.class);
749-
when(mock.order()).thenReturn(order);
750-
when(mock.getInterceptor()).thenReturn(Mockito.mock(ServerInterceptor.class));
751-
return mock;
750+
return new OrderedGrpcInterceptor() {
751+
752+
@Override
753+
public int order() {
754+
return order;
755+
}
756+
757+
@Override
758+
public ServerInterceptor getInterceptor() {
759+
return new ServerInterceptor() {
760+
@Override
761+
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
762+
ServerCall<ReqT, RespT> call,
763+
Metadata headers,
764+
ServerCallHandler<ReqT, RespT> next
765+
) {
766+
// no-op interceptor
767+
return next.startCall(call, headers);
768+
}
769+
};
770+
}
771+
};
752772
}
753773

754774
private void assertDoesNotThrow(Runnable runnable) {
@@ -901,7 +921,9 @@ public void testGrpcInterceptorChainIntegrationWithPlugin() {
901921
ThreadPool mockThreadPool = Mockito.mock(ThreadPool.class);
902922
when(mockThreadPool.getThreadContext()).thenReturn(new org.opensearch.common.util.concurrent.ThreadContext(Settings.EMPTY));
903923

904-
assertDoesNotThrow(() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, null, null, null, null, null));
924+
assertDoesNotThrow(
925+
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, environment, null, null, null, null)
926+
);
905927
}
906928

907929
public void testGrpcInterceptorChainWithDuplicateOrders() {
@@ -929,7 +951,7 @@ public void testGrpcInterceptorChainWithDuplicateOrders() {
929951
// Should throw exception due to duplicate orders during createComponents
930952
IllegalArgumentException exception = expectThrows(
931953
IllegalArgumentException.class,
932-
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, null, null, null, null, null)
954+
() -> plugin.createComponents(client, null, mockThreadPool, null, null, null, environment, null, null, null, null)
933955
);
934956

935957
// Verify error message includes order value and interceptor class names
@@ -984,6 +1006,55 @@ private void testGrpcInterceptorChain(List<OrderedGrpcInterceptor> interceptors,
9841006
}
9851007
}
9861008

1009+
public void testGrpcInterceptorProviderSettingsInitialization() {
1010+
// Mock extension loading for GrpcPlugin
1011+
TestSettingsAwareInterceptorProvider provider = new TestSettingsAwareInterceptorProvider();
1012+
ExtensiblePlugin.ExtensionLoader mockLoader = Mockito.mock(ExtensiblePlugin.ExtensionLoader.class);
1013+
when(mockLoader.loadExtensions(QueryBuilderProtoConverter.class)).thenReturn(null);
1014+
when(mockLoader.loadExtensions(GrpcInterceptorProvider.class)).thenReturn(List.of(provider));
1015+
GrpcPlugin plugin = new GrpcPlugin();
1016+
plugin.loadExtensions(mockLoader);
1017+
1018+
// Mock Environments
1019+
Settings validSetting = Settings.builder().put("test-setting", true).build();
1020+
Environment validEnv = Mockito.mock(Environment.class);
1021+
when(validEnv.settings()).thenReturn(validSetting);
1022+
1023+
Settings invalidSetting = Settings.builder().put("test-setting", false).build();
1024+
Environment invalidEnv = Mockito.mock(Environment.class);
1025+
when(invalidEnv.settings()).thenReturn(invalidSetting);
1026+
1027+
Settings emptySetting = Settings.builder().build();
1028+
Environment emptyEnv = Mockito.mock(Environment.class);
1029+
when(emptyEnv.settings()).thenReturn(emptySetting);
1030+
1031+
// createComponents initializes interceptor with the correct setting
1032+
assertDoesNotThrow(() -> plugin.createComponents(client, null, threadPool, null, null, null, validEnv, null, null, null, null));
1033+
1034+
// createComponents throws exception with the incorrect setting
1035+
try {
1036+
plugin.createComponents(client, null, threadPool, null, null, null, invalidEnv, null, null, null, null);
1037+
fail("Expect test interceptor with wrong settings throws exception.");
1038+
} catch (RuntimeException e) {
1039+
assertEquals("test-setting not found or not set to true", e.getMessage());
1040+
}
1041+
1042+
// createComponents throws exception with empty setting
1043+
try {
1044+
plugin.createComponents(client, null, threadPool, null, null, null, emptyEnv, null, null, null, null);
1045+
fail("Expect test interceptor with empty settings throws exception.");
1046+
} catch (RuntimeException e) {
1047+
assertEquals("test-setting not found or not set to true", e.getMessage());
1048+
}
1049+
}
1050+
1051+
public void testGrpcInterceptorProviderEmpty() {
1052+
GrpcInterceptorProvider prov = threadContext -> List.of();
1053+
assertDoesNotThrow(() -> prov.initNodeSettings(Settings.EMPTY));
1054+
List<OrderedGrpcInterceptor> interceptors = prov.getOrderedGrpcInterceptors(new ThreadContext(Settings.EMPTY));
1055+
assertTrue(interceptors.isEmpty());
1056+
}
1057+
9871058
/**
9881059
* Creates a test interceptor that can succeed or fail
9891060
*/
@@ -1040,4 +1111,45 @@ public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
10401111
};
10411112
}
10421113

1114+
/**
1115+
* Test interceptor provider that validates GrpcInterceptorProvider's access to settings.
1116+
* TestSettingsAwareInterceptorProvider will throw an exception if setting "test-setting" is not true.
1117+
*/
1118+
private static class TestSettingsAwareInterceptorProvider implements GrpcInterceptorProvider {
1119+
private Settings settings;
1120+
1121+
@Override
1122+
public void initNodeSettings(Settings settings) {
1123+
this.settings = settings;
1124+
}
1125+
1126+
@Override
1127+
public List<OrderedGrpcInterceptor> getOrderedGrpcInterceptors(ThreadContext threadContext) {
1128+
if (settings == null || !settings.getAsBoolean("test-setting", false)) {
1129+
throw new RuntimeException("test-setting not found or not set to true");
1130+
}
1131+
1132+
return List.of(new OrderedGrpcInterceptor() {
1133+
@Override
1134+
public int order() {
1135+
return 100;
1136+
}
1137+
1138+
@Override
1139+
public ServerInterceptor getInterceptor() {
1140+
return new ServerInterceptor() {
1141+
@Override
1142+
public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(
1143+
ServerCall<ReqT, RespT> call,
1144+
Metadata headers,
1145+
ServerCallHandler<ReqT, RespT> next
1146+
) {
1147+
// No-op interceptor - just pass through
1148+
return next.startCall(call, headers);
1149+
}
1150+
};
1151+
}
1152+
});
1153+
}
1154+
}
10431155
}

0 commit comments

Comments
 (0)