Skip to content

Commit 0257190

Browse files
tanik98tanik98
committed
Changing index setting name for reading from translog for derived source
Signed-off-by: Tanik Pansuriya <panbhai@amazon.com>
1 parent 2aef4c4 commit 0257190

File tree

8 files changed

+42
-126
lines changed

8 files changed

+42
-126
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
8282
- Added File Cache Pinning ([#17617](https://github.com/opensearch-project/OpenSearch/issues/13648))
8383
- Support consumer reset in Resume API for pull-based ingestion. This PR includes a breaking change for the experimental pull-based ingestion feature. ([#18332](https://github.com/opensearch-project/OpenSearch/pull/18332))
8484
- Add FIPS build tooling ([#4254](https://github.com/opensearch-project/security/issues/4254))
85+
- [Derived Source] Add integration of derived source feature across various paths like get/search/recovery ([#18565](https://github.com/opensearch-project/OpenSearch/pull/18565))
8586

8687
### Changed
8788
- Update Subject interface to use CheckedRunnable ([#18570](https://github.com/opensearch-project/OpenSearch/issues/18570))
@@ -141,6 +142,3 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
141142
### Security
142143

143144
[Unreleased 3.x]: https://github.com/opensearch-project/OpenSearch/compare/3.1...main
144-
### Added
145-
[Derive Source] Add integration of derived source feature across various paths like get/search/recovery #18565
146-
([#18565](https://github.com/opensearch-project/OpenSearch/pull/18565))

‎server/src/internalClusterTest/java/org/opensearch/get/GetActionIT.java‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@
5656
import org.opensearch.geometry.utils.Geohash;
5757
import org.opensearch.index.IndexSettings;
5858
import org.opensearch.index.engine.VersionConflictEngineException;
59-
import org.opensearch.index.translog.Translog;
6059
import org.opensearch.plugins.Plugin;
6160
import org.opensearch.test.InternalSettingsPlugin;
6261
import org.opensearch.test.OpenSearchIntegTestCase;
@@ -994,10 +993,7 @@ public void testDerivedSourceTranslogReadPreference() throws Exception {
994993
.put("index.number_of_replicas", 0)
995994
.put("index.refresh_interval", -1)
996995
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), true)
997-
.put(
998-
IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(),
999-
Translog.DerivedSourceReadPreference.DERIVED.name()
1000-
);
996+
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), true);
1001997

1002998
String mapping = XContentFactory.jsonBuilder()
1003999
.startObject()
@@ -1055,13 +1051,7 @@ public void testDerivedSourceTranslogReadPreference() throws Exception {
10551051
client().admin()
10561052
.indices()
10571053
.prepareUpdateSettings("test")
1058-
.setSettings(
1059-
Settings.builder()
1060-
.put(
1061-
IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(),
1062-
Translog.DerivedSourceReadPreference.SOURCE.name()
1063-
)
1064-
)
1054+
.setSettings(Settings.builder().put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), false))
10651055
.get()
10661056
);
10671057

‎server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
285285

286286
// Setting for derived source feature
287287
IndexSettings.INDEX_DERIVED_SOURCE_SETTING,
288-
IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING,
288+
IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING,
289289

290290
// validate that built-in similarities don't get redefined
291291
Setting.groupSetting("index.similarity.", (s) -> {

‎server/src/main/java/org/opensearch/index/IndexSettings.java‎

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -803,15 +803,9 @@ public static IndexMergePolicy fromString(String text) {
803803
Property.Final
804804
);
805805

806-
public static final Setting<String> INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING = new Setting<>(
807-
"index.derived_source.translog_read_preference",
808-
"",
809-
(value) -> {
810-
if (!Strings.isEmpty(value)) {
811-
return value.toUpperCase(Locale.ROOT);
812-
}
813-
return "";
814-
},
806+
public static final Setting<Boolean> INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING = Setting.boolSetting(
807+
"index.derived_source.translog.enabled",
808+
INDEX_DERIVED_SOURCE_SETTING,
815809
Property.IndexScope,
816810
Property.Dynamic
817811
);
@@ -867,7 +861,7 @@ public static IndexMergePolicy fromString(String text) {
867861
private final boolean isTranslogMetadataEnabled;
868862
private volatile boolean allowDerivedField;
869863
private final boolean derivedSourceEnabled;
870-
private volatile Translog.DerivedSourceReadPreference derivedSourceTranslogReadPreference;
864+
private volatile boolean derivedSourceEnabledForTranslog;
871865

872866
/**
873867
* The maximum age of a retention lease before it is considered expired.
@@ -1103,11 +1097,8 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
11031097
checkPendingFlushEnabled = scopedSettings.get(INDEX_CHECK_PENDING_FLUSH_ENABLED);
11041098
defaultSearchPipeline = scopedSettings.get(DEFAULT_SEARCH_PIPELINE);
11051099
derivedSourceEnabled = scopedSettings.get(INDEX_DERIVED_SOURCE_SETTING);
1106-
setDerivedSourceTranslogReadPreference(scopedSettings.get(INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING));
1107-
scopedSettings.addSettingsUpdateConsumer(
1108-
INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING,
1109-
this::setDerivedSourceTranslogReadPreference
1110-
);
1100+
derivedSourceEnabledForTranslog = scopedSettings.get(INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING);
1101+
scopedSettings.addSettingsUpdateConsumer(INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING, this::setDerivedSourceEnabledForTranslog);
11111102
/* There was unintentional breaking change got introduced with [OpenSearch-6424](https://github.com/opensearch-project/OpenSearch/pull/6424) (version 2.7).
11121103
* For indices created prior version (prior to 2.7) which has IndexSort type, they used to type cast the SortField.Type
11131104
* to higher bytes size like integer to long. This behavior was changed from OpenSearch 2.7 version not to
@@ -2143,33 +2134,21 @@ public void setRemoteStoreTranslogRepository(String remoteStoreTranslogRepositor
21432134
this.remoteStoreTranslogRepository = remoteStoreTranslogRepository;
21442135
}
21452136

2146-
private void setDerivedSourceTranslogReadPreference(String translogReadPreference) {
2147-
if (!Strings.isEmpty(translogReadPreference) && !isDerivedSourceEnabled()) {
2137+
private void setDerivedSourceEnabledForTranslog(boolean isDerivedSourceEnabledForTranslog) {
2138+
if (isDerivedSourceEnabledForTranslog && !isDerivedSourceEnabled()) {
21482139
throw new IllegalArgumentException(
21492140
"The "
2150-
+ IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey()
2141+
+ IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey()
21512142
+ " can't be set when "
21522143
+ IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey()
21532144
+ " setting is disabled"
21542145
);
21552146
}
2156-
if (Strings.isEmpty(translogReadPreference)) {
2157-
this.derivedSourceTranslogReadPreference = Translog.DerivedSourceReadPreference.DERIVED;
2158-
return;
2159-
}
2160-
try {
2161-
this.derivedSourceTranslogReadPreference = Translog.DerivedSourceReadPreference.valueOf(translogReadPreference);
2162-
} catch (IllegalArgumentException e) {
2163-
throw new IllegalArgumentException(
2164-
"The "
2165-
+ IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey()
2166-
+ " has unsupported setting value supplied"
2167-
);
2168-
}
2147+
this.derivedSourceEnabledForTranslog = isDerivedSourceEnabledForTranslog;
21692148
}
21702149

2171-
public Translog.DerivedSourceReadPreference getTranslogReadPreferenceForDerivedSource() {
2172-
return this.derivedSourceTranslogReadPreference;
2150+
public boolean isDerivedSourceEnabledForTranslog() {
2151+
return this.derivedSourceEnabledForTranslog;
21732152
}
21742153

21752154
public boolean isDerivedSourceEnabled() {

‎server/src/main/java/org/opensearch/index/engine/TranslogLeafReader.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,9 +323,7 @@ public void document(int docID, StoredFieldVisitor visitor) throws IOException {
323323
}
324324
if (visitor.needsField(FAKE_SOURCE_FIELD) == StoredFieldVisitor.Status.YES) {
325325
if (engineConfig.getIndexSettings().isDerivedSourceEnabled()
326-
&& Translog.DerivedSourceReadPreference.DERIVED.equals(
327-
engineConfig.getIndexSettings().getTranslogReadPreferenceForDerivedSource()
328-
)) {
326+
&& engineConfig.getIndexSettings().isDerivedSourceEnabledForTranslog()) {
329327
LeafReader leafReader = getInMemoryIndexReader();
330328
assert leafReader != null && leafReader.leaves().size() == 1;
331329
visitor.binaryField(

‎server/src/main/java/org/opensearch/index/translog/Translog.java‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2145,24 +2145,4 @@ public long getMinUnreferencedSeqNoInSegments(long minUnrefCheckpointInLastCommi
21452145
protected boolean shouldFlush() {
21462146
return false;
21472147
}
2148-
2149-
/**
2150-
* How to read source from the translog for get operations under derived source feature
2151-
*
2152-
* @opensearch.api
2153-
*/
2154-
@PublicApi(since = "3.1.0")
2155-
public enum DerivedSourceReadPreference {
2156-
2157-
/**
2158-
* Derived source, inferred source might defer from original ingested source in terms of display form but
2159-
* contextually it will be similar to original ingested source
2160-
*/
2161-
DERIVED,
2162-
2163-
/**
2164-
* Original ingested source, which would be returned as it is
2165-
*/
2166-
SOURCE
2167-
}
21682148
}

‎server/src/test/java/org/opensearch/index/IndexSettingsTests.java‎

Lines changed: 23 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,32 +1045,23 @@ public void testDerivedSourceTranslogReadPreferenceValidation() {
10451045
Settings.builder()
10461046
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
10471047
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), true)
1048-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "DERIVED")
1048+
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), true)
10491049
.build()
10501050
);
10511051
IndexSettings settings = new IndexSettings(metadata, Settings.EMPTY);
1052-
assertEquals(Translog.DerivedSourceReadPreference.DERIVED, settings.getTranslogReadPreferenceForDerivedSource());
1052+
assertTrue(settings.isDerivedSourceEnabledForTranslog());
10531053

10541054
// Test 2: Invalid case - setting read preference when derived source is disabled
1055-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
1056-
IndexMetadata invalidMetadata = newIndexMeta(
1057-
"index",
1058-
Settings.builder()
1059-
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
1060-
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false)
1061-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "source")
1062-
.build()
1063-
);
1064-
new IndexSettings(invalidMetadata, Settings.EMPTY);
1065-
});
1066-
assertEquals(
1067-
"The "
1068-
+ IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey()
1069-
+ " can't be set when "
1070-
+ IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey()
1071-
+ " setting is disabled",
1072-
e.getMessage()
1055+
metadata = newIndexMeta(
1056+
"index",
1057+
Settings.builder()
1058+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
1059+
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false)
1060+
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), false)
1061+
.build()
10731062
);
1063+
settings = new IndexSettings(metadata, Settings.EMPTY);
1064+
assertFalse(settings.isDerivedSourceEnabledForTranslog());
10741065

10751066
// Test 3: Default(Derived) behavior - no read preference set
10761067
metadata = newIndexMeta(
@@ -1081,49 +1072,29 @@ public void testDerivedSourceTranslogReadPreferenceValidation() {
10811072
.build()
10821073
);
10831074
settings = new IndexSettings(metadata, Settings.EMPTY);
1084-
assertEquals(Translog.DerivedSourceReadPreference.DERIVED, settings.getTranslogReadPreferenceForDerivedSource());
1075+
assertTrue(settings.isDerivedSourceEnabledForTranslog());
10851076

10861077
// Test 4: Dynamic update - valid case
10871078
settings.updateIndexMetadata(
10881079
newIndexMeta(
10891080
"index",
10901081
Settings.builder()
10911082
.put(metadata.getSettings())
1092-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "source")
1083+
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), false)
10931084
.build()
10941085
)
10951086
);
1096-
assertEquals(Translog.DerivedSourceReadPreference.SOURCE, settings.getTranslogReadPreferenceForDerivedSource());
1087+
assertFalse(settings.isDerivedSourceEnabledForTranslog());
10971088

1098-
// Test 5: Empty value should default to DERIVED
1099-
settings.updateIndexMetadata(
1100-
newIndexMeta(
1101-
"index",
1102-
Settings.builder()
1103-
.put(metadata.getSettings())
1104-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "")
1105-
.build()
1106-
)
1107-
);
1108-
assertEquals(Translog.DerivedSourceReadPreference.DERIVED, settings.getTranslogReadPreferenceForDerivedSource());
1109-
1110-
// Test 6: Invalid setting name
1111-
e = expectThrows(IllegalArgumentException.class, () -> {
1112-
IndexMetadata invalidMetadata = newIndexMeta(
1113-
"index",
1114-
Settings.builder()
1115-
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
1116-
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), true)
1117-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "invalid setting")
1118-
.build()
1119-
);
1120-
new IndexSettings(invalidMetadata, Settings.EMPTY);
1121-
});
1122-
assertEquals(
1123-
"The "
1124-
+ IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey()
1125-
+ " has unsupported setting value supplied",
1126-
e.getMessage()
1089+
// Test 5: If derived source is disabled then translog setting value should also be disabled
1090+
metadata = newIndexMeta(
1091+
"index",
1092+
Settings.builder()
1093+
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT)
1094+
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), false)
1095+
.build()
11271096
);
1097+
settings = new IndexSettings(metadata, Settings.EMPTY);
1098+
assertFalse(settings.isDerivedSourceEnabledForTranslog());
11281099
}
11291100
}

‎server/src/test/java/org/opensearch/index/engine/TranslogLeafReaderTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ public void testDerivedSourceFieldsUsingDerivedSource() throws IOException {
176176
// Setup mapper service with derived source enabled
177177
Settings derivedSourceSettings = Settings.builder()
178178
.put(defaultIndexSettings.getSettings())
179-
.put("index.derived_source.enabled", true)
179+
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), true)
180180
.build();
181181
IndexMetadata derivedMetadata = IndexMetadata.builder("test").settings(derivedSourceSettings).build();
182182
IndexSettings derivedIndexSettings = new IndexSettings(derivedMetadata, Settings.EMPTY);
@@ -219,7 +219,7 @@ public void testDerivedSourceFieldsUsingSource() throws IOException {
219219
Settings derivedSourceSettings = Settings.builder()
220220
.put(defaultIndexSettings.getSettings())
221221
.put(IndexSettings.INDEX_DERIVED_SOURCE_SETTING.getKey(), true)
222-
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_READ_PREFERENCE_SETTING.getKey(), "source")
222+
.put(IndexSettings.INDEX_DERIVED_SOURCE_TRANSLOG_ENABLED_SETTING.getKey(), false)
223223
.build();
224224
IndexMetadata derivedMetadata = IndexMetadata.builder("test").settings(derivedSourceSettings).build();
225225
IndexSettings derivedIndexSettings = new IndexSettings(derivedMetadata, Settings.EMPTY);

0 commit comments

Comments
 (0)