Skip to content

Commit

Permalink
Rename translog pruning setting to CCR specific setting and addressed…
Browse files Browse the repository at this point in the history
… Bug in the test case (#1243)

* Rename translog pruing setting to CCR specific setting

Signed-off-by: Sai Kumar <karanas@amazon.com>

* Rename to index.plugins.replication.translog.retention_lease.pruning.enabled as
index settings needs "index." as prefix

Signed-off-by: Sai Kumar <karanas@amazon.com>

* Add deprecations to retention pruning controls

This commit adds deprecation flags to all added settings, variables, and methods
specific to ccr's retention lease pruning mechanism.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>

* Addressed CR comments

Signed-off-by: Sai Kumar <karanas@amazon.com>

* fix javadoc deprecation

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>

* fix deprecation tag in TranslogDeletionPolicy

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>

* Addressed test issue under translog tests

Signed-off-by: Sai Kumar <karanas@amazon.com>

Co-authored-by: Nicholas Walter Knize <nknize@apache.org>
  • Loading branch information
saikaranam-amazon and nknize authored Sep 24, 2021
1 parent 82d1d0e commit 29c88c6
Show file tree
Hide file tree
Showing 9 changed files with 96 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,33 @@ public void testNumberOfReplicasSettingsVersion() {
assertThat(newSettingsVersion, equalTo(1 + settingsVersion));
}

public void testTranslogPruningSetting() {
createIndex("test");
ensureGreen("test");
final String REPLICATION_TRANSLOG_SETTING = "index.plugins.replication.translog.retention_lease.pruning.enabled";
final long settingsVersion =
client().admin().cluster().prepareState().get().getState().metadata().index("test").getSettingsVersion();
GetSettingsResponse settingsResponse = client().admin().indices().prepareGetSettings("test").get();
boolean shouldPruneTranslogByRetentionLease = Boolean.parseBoolean(
settingsResponse.getSetting("test", REPLICATION_TRANSLOG_SETTING)
);

assertFalse(shouldPruneTranslogByRetentionLease);
assertAcked(client().admin().indices()
.prepareUpdateSettings("test")
.setSettings(Settings.builder()
.put("index.plugins.replication.translog.retention_lease.pruning.enabled", true)
));
final long newSettingsVersion =
client().admin().cluster().prepareState().get().getState().metadata().index("test").getSettingsVersion();
assertThat(newSettingsVersion, equalTo(1 + settingsVersion));
settingsResponse = client().admin().indices().prepareGetSettings("test").get();
shouldPruneTranslogByRetentionLease = Boolean.parseBoolean(
settingsResponse.getSetting("test", REPLICATION_TRANSLOG_SETTING)
);
assertTrue(shouldPruneTranslogByRetentionLease);
}

/*
* Test that we are able to set the setting index.number_of_replicas to the default.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
EnableAllocationDecider.INDEX_ROUTING_REBALANCE_ENABLE_SETTING,
EnableAllocationDecider.INDEX_ROUTING_ALLOCATION_ENABLE_SETTING,
IndexSettings.INDEX_FLUSH_AFTER_MERGE_THRESHOLD_SIZE_SETTING,
IndexSettings.INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING,
IndexSettings.INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING,
IndexSettings.INDEX_TRANSLOG_FLUSH_THRESHOLD_SIZE_SETTING,
IndexSettings.INDEX_TRANSLOG_GENERATION_THRESHOLD_SIZE_SETTING,
IndexSettings.INDEX_TRANSLOG_RETENTION_AGE_SETTING,
Expand Down
33 changes: 24 additions & 9 deletions server/src/main/java/org/opensearch/index/IndexSettings.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,10 +262,12 @@ public final class IndexSettings {

/**
* Specifies if the index translog should prune based on retention leases.
* @deprecated EXPERT: this setting is specific to CCR and will be moved to a plugin in the next release
*/
public static final Setting<Boolean> INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING =
Setting.boolSetting("index.translog.retention_lease.pruning.enabled", false,
Property.IndexScope, Property.Dynamic);
@Deprecated
public static final Setting<Boolean> INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING =
Setting.boolSetting("index.plugins.replication.translog.retention_lease.pruning.enabled", false,
Property.IndexScope, Property.Dynamic, Property.Deprecated);

/**
* Controls how many soft-deleted documents will be kept around before being merged away. Keeping more deleted
Expand Down Expand Up @@ -293,6 +295,7 @@ public final class IndexSettings {
* the chance of ops based recoveries for indices with soft-deletes disabled.
* This setting will be ignored if soft-deletes is used in peer recoveries (default in 7.4).
**/
@Deprecated
private static final ByteSizeValue DEFAULT_TRANSLOG_RETENTION_SIZE = new ByteSizeValue(512, ByteSizeUnit.MB);

public static final Setting<ByteSizeValue> INDEX_TRANSLOG_RETENTION_SIZE_SETTING =
Expand Down Expand Up @@ -398,6 +401,7 @@ public final class IndexSettings {
private final IndexScopedSettings scopedSettings;
private long gcDeletesInMillis = DEFAULT_GC_DELETES.millis();
private final boolean softDeleteEnabled;
@Deprecated
private volatile boolean translogPruningByRetentionLease;
private volatile long softDeleteRetentionOperations;

Expand Down Expand Up @@ -535,9 +539,10 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
mergeSchedulerConfig = new MergeSchedulerConfig(this);
gcDeletesInMillis = scopedSettings.get(INDEX_GC_DELETES_SETTING).getMillis();
softDeleteEnabled = version.onOrAfter(LegacyESVersion.V_6_5_0) && scopedSettings.get(INDEX_SOFT_DELETES_SETTING);
translogPruningByRetentionLease = version.onOrAfter(Version.V_1_1_0) &&
// @todo move to CCR plugin
this.translogPruningByRetentionLease = version.onOrAfter(Version.V_1_1_0) &&
softDeleteEnabled &&
scopedSettings.get(INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING);
scopedSettings.get(INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING);
softDeleteRetentionOperations = scopedSettings.get(INDEX_SOFT_DELETES_RETENTION_OPERATIONS_SETTING);
retentionLeaseMillis = scopedSettings.get(INDEX_SOFT_DELETES_RETENTION_LEASE_PERIOD_SETTING).millis();
warmerEnabled = scopedSettings.get(INDEX_WARMER_ENABLED_SETTING);
Expand Down Expand Up @@ -606,7 +611,7 @@ public IndexSettings(final IndexMetadata indexMetadata, final Settings nodeSetti
this::setGenerationThresholdSize);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_RETENTION_AGE_SETTING, this::setTranslogRetentionAge);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_RETENTION_SIZE_SETTING, this::setTranslogRetentionSize);
scopedSettings.addSettingsUpdateConsumer(INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING,
scopedSettings.addSettingsUpdateConsumer(INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING,
this::setTranslogPruningByRetentionLease);
scopedSettings.addSettingsUpdateConsumer(INDEX_REFRESH_INTERVAL_SETTING, this::setRefreshInterval);
scopedSettings.addSettingsUpdateConsumer(MAX_REFRESH_LISTENERS_PER_SHARD, this::setMaxRefreshListeners);
Expand Down Expand Up @@ -638,13 +643,19 @@ private void setFlushAfterMergeThresholdSize(ByteSizeValue byteSizeValue) {
this.flushAfterMergeThresholdSize = byteSizeValue;
}

/**
* Enable or disable translog pruning by retention lease requirement
*
* @deprecated EXPERT: this setting is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
private void setTranslogPruningByRetentionLease(boolean enabled) {
this.translogPruningByRetentionLease = INDEX_SOFT_DELETES_SETTING.get(settings) && enabled;
}

private void setTranslogRetentionSize(ByteSizeValue byteSizeValue) {
if (shouldDisableTranslogRetention(settings) &&
!shouldPruneTranslogByRetentionLease(settings) &&
shouldPruneTranslogByRetentionLease(settings) == false &&
byteSizeValue.getBytes() >= 0) {
// ignore the translog retention settings if soft-deletes enabled
this.translogRetentionSize = new ByteSizeValue(-1);
Expand Down Expand Up @@ -847,7 +858,7 @@ public TimeValue getRefreshInterval() {
* Returns the transaction log retention size which controls how much of the translog is kept around to allow for ops based recoveries
*/
public ByteSizeValue getTranslogRetentionSize() {
if(shouldDisableTranslogRetention(settings) && !shouldPruneTranslogByRetentionLease(settings)) {
if(shouldDisableTranslogRetention(settings) && shouldPruneTranslogByRetentionLease(settings) == false) {
return new ByteSizeValue(-1);
}
else if(shouldPruneTranslogByRetentionLease(settings) && translogRetentionSize.getBytes() == -1) {
Expand Down Expand Up @@ -1099,16 +1110,20 @@ public void setRequiredPipeline(final String requiredPipeline) {

/**
* Returns <code>true</code> if translog ops should be pruned based on retention lease
* @deprecated EXPERT: this setting is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
public boolean shouldPruneTranslogByRetentionLease() {
return translogPruningByRetentionLease;
}

/**
* Returns <code>true</code> if translog ops should be pruned based on retention lease
* @deprecated EXPERT: this setting is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
public static boolean shouldPruneTranslogByRetentionLease(Settings settings) {
return INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.get(settings);
return INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.get(settings);
}

/**
Expand Down
8 changes: 7 additions & 1 deletion server/src/main/java/org/opensearch/index/engine/Engine.java
Original file line number Diff line number Diff line change
Expand Up @@ -1861,9 +1861,15 @@ public void onSettingsChanged(TimeValue translogRetentionAge, ByteSizeValue tran
onSettingsChanged(translogRetentionAge, translogRetentionSize, softDeletesRetentionOps, false);
}

/**
* callback when index settings change
*
* @deprecated EXPERT: this method is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
public void onSettingsChanged(TimeValue translogRetentionAge, ByteSizeValue translogRetentionSize,
long softDeletesRetentionOps, boolean translogPruningByRetentionLease) {

// @todo this is overly silent; make this abstract and force derived classes to noop in the next release
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ final Checkpoint getLastSyncedCheckpoint() {

// for testing
public Snapshot newSnapshot() throws IOException {
return newSnapshot(0, Long.MAX_VALUE, false);
return newSnapshot(0, Long.MAX_VALUE);
}

public Snapshot newSnapshot(long fromSeqNo, long toSeqNo) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@
public class TranslogDeletionPolicy {

private final Map<Object, RuntimeException> openTranslogRef;
/**
* @deprecated EXPERT: this supplier is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
private Supplier<RetentionLeases> retentionLeasesSupplier;

public void assertNoOpenTranslogRefs() {
Expand All @@ -73,6 +77,10 @@ public void assertNoOpenTranslogRefs() {

private int retentionTotalFiles;

/**
* @deprecated EXPERT: this variable is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
private boolean shouldPruneTranslogByRetentionLease;

public TranslogDeletionPolicy(long retentionSizeInBytes, long retentionAgeInMillis, int retentionTotalFiles) {
Expand All @@ -86,6 +94,11 @@ public TranslogDeletionPolicy(long retentionSizeInBytes, long retentionAgeInMill
}
}

/**
* Construct a TranslogDeletionPolicy to include pruning by retention leases
* @deprecated EXPERT: this ctor is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
public TranslogDeletionPolicy(long retentionSizeInBytes, long retentionAgeInMillis, int retentionTotalFiles,
Supplier<RetentionLeases> retentionLeasesSupplier) {
this(retentionSizeInBytes, retentionAgeInMillis, retentionTotalFiles);
Expand All @@ -112,6 +125,11 @@ synchronized void setRetentionTotalFiles(int retentionTotalFiles) {
this.retentionTotalFiles = retentionTotalFiles;
}

/**
* Should the translog be pruned by the retention lease heuristic
* @deprecated EXPERT: this setting is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
public synchronized void shouldPruneTranslogByRetentionLease(boolean translogPruneByRetentionLease) {
this.shouldPruneTranslogByRetentionLease = translogPruneByRetentionLease;
}
Expand Down Expand Up @@ -191,6 +209,11 @@ synchronized long minTranslogGenRequired(List<TranslogReader> readers, TranslogW
return Math.min(minByTranslogGenSettings, minByRetentionLeasesAndSize);
}

/**
* Find the minimum translog generation by minimum retaining sequence number
* @deprecated EXPERT: this configuration is specific to CCR and will be moved to a plugin in the next release
*/
@Deprecated
static long getMinTranslogGenByRetentionLease(List<TranslogReader> readers, TranslogWriter writer,
Supplier<RetentionLeases> retentionLeasesSupplier) {
long minGen = writer.getGeneration();
Expand Down
12 changes: 10 additions & 2 deletions server/src/test/java/org/opensearch/index/IndexSettingsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,8 @@ public void testTranslogPruningSettingsWithSoftDeletesEnabled() {

ByteSizeValue retentionSize = new ByteSizeValue(512, ByteSizeUnit.MB);
boolean translogPruningEnabled = randomBoolean();
settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.getKey(), translogPruningEnabled);
settings.put(IndexSettings.INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.getKey(),
translogPruningEnabled);
IndexMetadata metadata = newIndexMeta("index", settings.build());
IndexSettings indexSettings = new IndexSettings(metadata, Settings.EMPTY);
if(translogPruningEnabled) {
Expand All @@ -672,6 +673,9 @@ public void testTranslogPruningSettingsWithSoftDeletesEnabled() {
assertFalse(indexSettings.shouldPruneTranslogByRetentionLease());
assertThat(indexSettings.getTranslogRetentionSize().getBytes(), equalTo(-1L));
}
assertWarnings("[index.plugins.replication.translog.retention_lease.pruning.enabled] setting " +
"was deprecated in OpenSearch and will be removed in a future release! See the breaking changes documentation " +
"for the next major version.");
}

public void testTranslogPruningSettingsWithSoftDeletesDisabled() {
Expand All @@ -680,10 +684,14 @@ public void testTranslogPruningSettingsWithSoftDeletesDisabled() {
.put(IndexMetadata.SETTING_VERSION_CREATED, Version.CURRENT);
boolean translogPruningEnabled = randomBoolean();
ByteSizeValue retentionSize = new ByteSizeValue(512, ByteSizeUnit.MB);
settings.put(IndexSettings.INDEX_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.getKey(), translogPruningEnabled);
settings.put(IndexSettings.INDEX_PLUGINS_REPLICATION_TRANSLOG_RETENTION_LEASE_PRUNING_ENABLED_SETTING.getKey(),
translogPruningEnabled);
IndexMetadata metadata = newIndexMeta("index", settings.build());
IndexSettings indexSettings = new IndexSettings(metadata, Settings.EMPTY);
assertFalse(indexSettings.shouldPruneTranslogByRetentionLease());
assertThat(indexSettings.getTranslogRetentionSize().getBytes(), equalTo(retentionSize.getBytes()));
assertWarnings("[index.plugins.replication.translog.retention_lease.pruning.enabled] setting " +
"was deprecated in OpenSearch and will be removed in a future release! See the breaking changes documentation " +
"for the next major version.");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public void testWithRetentionLease() throws IOException {
List<BaseTranslogReader> allGens = new ArrayList<>(readersAndWriter.v1());
allGens.add(readersAndWriter.v2());
Supplier<RetentionLeases> retentionLeasesSupplier = createRetentionLeases(now, 0L,
readersAndWriter.v1().size() * TOTAL_OPS_IN_GEN - 1);
allGens.size() * TOTAL_OPS_IN_GEN - 1);
try {
final long minimumRetainingSequenceNumber = retentionLeasesSupplier.get()
.leases()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ public void testFullRangeSnapshot() throws Exception {
// Successful snapshot
long nextSeqNo = populateTranslogOps(false);
long fromSeqNo = 0L;
long toSeqNo = Math.max(nextSeqNo - 1, fromSeqNo + 15);
long toSeqNo = Math.min(nextSeqNo - 1, fromSeqNo + 15);
try (Translog.Snapshot snapshot = translog.newSnapshot(fromSeqNo, toSeqNo, true)) {
int totOps = 0;
for (Translog.Operation op = snapshot.next(); op != null; op = snapshot.next()) {
Expand All @@ -793,7 +793,7 @@ public void testFullRangeSnapshot() throws Exception {
public void testFullRangeSnapshotWithFailures() throws Exception {
long nextSeqNo = populateTranslogOps(true);
long fromSeqNo = 0L;
long toSeqNo = Math.max(nextSeqNo-1, fromSeqNo + 15);
long toSeqNo = Math.min(nextSeqNo-1, fromSeqNo + 15);
try (Translog.Snapshot snapshot = translog.newSnapshot(fromSeqNo, toSeqNo, true)) {
int totOps = 0;
for (Translog.Operation op = snapshot.next(); op != null; op = snapshot.next()) {
Expand Down

0 comments on commit 29c88c6

Please sign in to comment.