From 57f1264a1a15d9c55e242734d239172727e60246 Mon Sep 17 00:00:00 2001 From: Przemyslaw Witek Date: Tue, 6 Feb 2024 14:58:48 +0100 Subject: [PATCH 1/3] [Transform] Allow using PIT in the presence of remote clusters --- .../transforms/ClientTransformIndexer.java | 3 +- .../ClientTransformIndexerTests.java | 70 +++---------------- 2 files changed, 9 insertions(+), 64 deletions(-) diff --git a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexer.java b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexer.java index 55f0290c20a1c..1f9ec86ada8e2 100644 --- a/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexer.java +++ b/x-pack/plugin/transform/src/main/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexer.java @@ -449,8 +449,7 @@ private void injectPointInTimeIfNeeded( ActionListener> listener ) { SearchRequest searchRequest = namedSearchRequest.v2(); - // We explicitly disable PIT in the presence of remote clusters in the source due to huge PIT handles causing performance problems. - if (disablePit || searchRequest.indices().length == 0 || transformConfig.getSource().requiresRemoteCluster()) { + if (disablePit || searchRequest.indices().length == 0) { listener.onResponse(namedSearchRequest); return; } diff --git a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexerTests.java b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexerTests.java index fa8e867d77a49..1c6d1615cbb9c 100644 --- a/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexerTests.java +++ b/x-pack/plugin/transform/src/test/java/org/elasticsearch/xpack/transform/transforms/ClientTransformIndexerTests.java @@ -293,8 +293,14 @@ public void testPitInjectionIfPitNotSupported() throws InterruptedException { } public void testDisablePit() throws InterruptedException { - // TransformConfigTests.randomTransformConfig never produces remote indices in the source, hence we are safe here. */ - TransformConfig config = TransformConfigTests.randomTransformConfig(); + TransformConfig.Builder configBuilder = new TransformConfig.Builder(TransformConfigTests.randomTransformConfig()); + if (randomBoolean()) { + // TransformConfigTests.randomTransformConfig never produces remote indices in the source. + // We need to explicitly set the remote index here for coverage. + configBuilder.setSource(new SourceConfig("remote-cluster:remote-index")); + } + TransformConfig config = configBuilder.build(); + boolean pitEnabled = config.getSettings().getUsePit() == null || config.getSettings().getUsePit(); try (var threadPool = createThreadPool()) { @@ -354,66 +360,6 @@ public void testDisablePit() throws InterruptedException { } } - public void testDisablePitWhenThereIsRemoteIndexInSource() throws InterruptedException { - TransformConfig config = new TransformConfig.Builder(TransformConfigTests.randomTransformConfig()) - // Remote index is configured within source - .setSource(new SourceConfig("remote-cluster:remote-index")) - .build(); - boolean pitEnabled = config.getSettings().getUsePit() == null || config.getSettings().getUsePit(); - - try (var threadPool = createThreadPool()) { - final var client = new PitMockClient(threadPool, true); - MockClientTransformIndexer indexer = new MockClientTransformIndexer( - mock(ThreadPool.class), - new TransformServices( - mock(IndexBasedTransformConfigManager.class), - mock(TransformCheckpointService.class), - mock(TransformAuditor.class), - new TransformScheduler(Clock.systemUTC(), mock(ThreadPool.class), Settings.EMPTY, TimeValue.ZERO) - ), - mock(CheckpointProvider.class), - new AtomicReference<>(IndexerState.STOPPED), - null, - new ParentTaskAssigningClient(client, new TaskId("dummy-node:123456")), - mock(TransformIndexerStats.class), - config, - null, - new TransformCheckpoint( - "transform", - Instant.now().toEpochMilli(), - 0L, - Collections.emptyMap(), - Instant.now().toEpochMilli() - ), - new TransformCheckpoint( - "transform", - Instant.now().toEpochMilli(), - 2L, - Collections.emptyMap(), - Instant.now().toEpochMilli() - ), - new SeqNoPrimaryTermAndIndex(1, 1, TransformInternalIndexConstants.LATEST_INDEX_NAME), - mock(TransformContext.class), - false - ); - - // Because remote index is configured within source, we expect PIT *not* being used regardless the transform settings - this.assertAsync( - listener -> indexer.doNextSearch(0, listener), - response -> assertNull(response.pointInTimeId()) - ); - - // reverse the setting - indexer.applyNewSettings(new SettingsConfig.Builder().setUsePit(pitEnabled == false).build()); - - // Because remote index is configured within source, we expect PIT *not* being used regardless the transform settings - this.assertAsync( - listener -> indexer.doNextSearch(0, listener), - response -> assertNull(response.pointInTimeId()) - ); - } - } - public void testHandlePitIndexNotFound() throws InterruptedException { // simulate a deleted index due to ILM try (var threadPool = createThreadPool()) { From 5e623927b90725d9868e5f5a20c48a0d9cedd5c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Witek?= Date: Tue, 6 Feb 2024 16:10:03 +0100 Subject: [PATCH 2/3] Update docs/changelog/105192.yaml --- docs/changelog/105192.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/105192.yaml diff --git a/docs/changelog/105192.yaml b/docs/changelog/105192.yaml new file mode 100644 index 0000000000000..9d057dd09b06e --- /dev/null +++ b/docs/changelog/105192.yaml @@ -0,0 +1,6 @@ +pr: 105192 +summary: Allow transforms to use PIT with remote clusters again +area: Transform +type: bug +issues: + - 104518 From 634be7871b0b1b7447d59a6c8eb9c6181cc3cae3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Przemys=C5=82aw=20Witek?= Date: Tue, 6 Feb 2024 16:10:37 +0100 Subject: [PATCH 3/3] Update docs/changelog/105192.yaml --- docs/changelog/105192.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/105192.yaml b/docs/changelog/105192.yaml index 9d057dd09b06e..b15d58ef40fe7 100644 --- a/docs/changelog/105192.yaml +++ b/docs/changelog/105192.yaml @@ -1,6 +1,6 @@ pr: 105192 summary: Allow transforms to use PIT with remote clusters again area: Transform -type: bug +type: enhancement issues: - 104518