Skip to content

Commit

Permalink
fix: encapsulate translog options into translog in index settings
Browse files Browse the repository at this point in the history 
Now that all the translog options are stored in a separate object, we
should remove the separate fields to avoid confusion.

Public methods (getters, builder setters) should remain intact, mark
them deprecated and point them to the corresponding translog fields.

Deserializer should still recognize string-valued translog attributes, keep
the deprecated setters as field deserializers.

Serializer does not have to produce string-valued translog attributes,
as server parses `translog` attribute. Do not write translog contents to
old `translog.xxx` fields.

Signed-off-by: Maksim Strutovskii <[email protected]>
  • Loading branch information
@Mstrutov
Mstrutov committed Jun 13, 2023
1 parent 6ba68d8 commit 8d06598
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 81 deletions.
93 changes: 30 additions & 63 deletions java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,21 @@

package org.opensearch.client.opensearch.indices;

import org.opensearch.client.opensearch._types.Time;
import java.util.List;
import java.util.function.Function;
import javax.annotation.Nullable;

import jakarta.json.stream.JsonGenerator;
import org.opensearch.client.json.JsonpDeserializable;
import org.opensearch.client.json.JsonpDeserializer;
import org.opensearch.client.json.JsonpMapper;
import org.opensearch.client.json.JsonpSerializable;
import org.opensearch.client.json.ObjectBuilderDeserializer;
import org.opensearch.client.json.ObjectDeserializer;
import org.opensearch.client.opensearch._types.Time;
import org.opensearch.client.util.ApiTypeHelper;
import org.opensearch.client.util.ObjectBuilder;
import org.opensearch.client.util.ObjectBuilderBase;
import jakarta.json.stream.JsonGenerator;
import java.util.List;
import java.util.function.Function;
import javax.annotation.Nullable;

// typedef: indices._types.IndexSettings

Expand Down Expand Up @@ -201,15 +202,6 @@ public class IndexSettings implements JsonpSerializable {
@Nullable
private final Translog translog;

@Nullable
private final String translogDurability;

@Nullable
private final String translogFlushThresholdSize;

@Nullable
private final Time translogSyncInterval;

@Nullable
private final Boolean queryStringLenient;

Expand Down Expand Up @@ -281,9 +273,6 @@ private IndexSettings(Builder builder) {
this.format = builder.format;
this.maxSlicesPerScroll = builder.maxSlicesPerScroll;
this.translog = builder.translog;
this.translogDurability = builder.translogDurability;
this.translogFlushThresholdSize = builder.translogFlushThresholdSize;
this.translogSyncInterval = builder.translogSyncInterval;
this.queryStringLenient = builder.queryStringLenient;
this.priority = builder.priority;
this.topMetricsMaxSize = builder.topMetricsMaxSize;
Expand Down Expand Up @@ -690,26 +679,22 @@ public final Translog translog() {

/**
* API name: {@code translog.durability}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
@Nullable
public final String translogDurability() {
return this.translogDurability;
return this.translog != null ? this.translog.durability() : null;
}

/**
* API name: {@code translog.flush_threshold_size}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
@Nullable
public final String translogFlushThresholdSize() {
return this.translogFlushThresholdSize;
}

/**
* API name: {@code translog.sync_interval}
*/
@Nullable
public final Time translogSyncInterval() {
return this.translogSyncInterval;
return this.translog != null ? this.translog.flushThresholdSize() : null;
}

/**
Expand Down Expand Up @@ -1020,21 +1005,6 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {
generator.writeKey("translog");
this.translog.serialize(generator, mapper);

}
if (this.translogDurability != null) {
generator.writeKey("translog.durability");
generator.write(this.translogDurability);

}
if (this.translogFlushThresholdSize != null) {
generator.writeKey("translog.flush_threshold_size");
generator.write(this.translogFlushThresholdSize);

}
if (this.translogSyncInterval != null) {
generator.writeKey("translog.sync_interval");
this.translogSyncInterval.serialize(generator, mapper);

}
if (this.queryStringLenient != null) {
generator.writeKey("query_string.lenient");
Expand Down Expand Up @@ -1223,15 +1193,6 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder<I
@Nullable
private Translog translog;

@Nullable
private String translogDurability;

@Nullable
private String translogFlushThresholdSize;

@Nullable
private Time translogSyncInterval;

@Nullable
private Boolean queryStringLenient;

Expand Down Expand Up @@ -1726,25 +1687,33 @@ public final Builder translog(@Nullable Translog value) {

/**
* API name: {@code translog.durability}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
public final Builder translogDurability(@Nullable String value) {
this.translogDurability = value;
if (translog == null) {
translog = Translog.of(b -> b.durability(value));
} else {
translog = Translog.of(b -> b.durability(value)
.flushThresholdSize(translog.flushThresholdSize())
.syncInterval(translog.syncInterval()));
}
return this;
}

/**
* API name: {@code translog.flush_threshold_size}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
public final Builder translogFlushThresholdSize(@Nullable String value) {
this.translogFlushThresholdSize = value;
return this;
}

/**
* API name: {@code translog.sync_interval}
*/
public final Builder translogSyncInterval(@Nullable Time value) {
this.translogSyncInterval = value;
if (translog == null) {
translog = Translog.of(b -> b.flushThresholdSize(value));
} else {
translog = Translog.of(b -> b.flushThresholdSize(value)
.durability(translog.durability())
.syncInterval(translog.syncInterval()));
}
return this;
}

Expand Down Expand Up @@ -1924,8 +1893,6 @@ protected static void setupIndexSettingsDeserializer(ObjectDeserializer<IndexSet
"index.translog.durability");
op.add(Builder::translogFlushThresholdSize, JsonpDeserializer.stringDeserializer(),
"translog.flush_threshold_size", "index.translog.flush_threshold_size");
op.add(Builder::translogSyncInterval, Time._DESERIALIZER,
"translog.sync_interval", "index.translog.sync_interval");
op.add(Builder::queryStringLenient, JsonpDeserializer.booleanDeserializer(), "query_string.lenient",
"index.query_string.lenient");
op.add(Builder::priority, JsonpDeserializer.stringDeserializer(), "priority", "index.priority");
Expand Down
26 changes: 18 additions & 8 deletions java-client/src/test/java/org/opensearch/client/opensearch/experiments/ParsingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.opensearch.client.opensearch.experiments.api.FooRequest;
import org.opensearch.client.opensearch.indices.IndexSettings;
import org.opensearch.client.opensearch.indices.IndexSettingsMapping;
import org.opensearch.client.opensearch.indices.Translog;

public class ParsingTests extends Assert {

Expand Down Expand Up @@ -79,21 +80,30 @@ public void testFoo() throws Exception {
}

@Test
public void testIndexSettingsSyncIntervalTimeParsing() {
public void testIndexSettingsTranslogOptionsParsing() {

try {

var indexSettings = IndexSettings.of(_1 -> _1.translogSyncInterval(Time.of(_2 -> _2.time("10s"))));
var indexSettings = IndexSettings.of(_1 -> _1.translog(Translog.of(tr -> tr
.syncInterval(Time.of(t -> t.time("10s")))
.durability("async")
.flushThresholdSize("256mb"))));

var str = serialize(indexSettings);
assertEquals("{\"translog.sync_interval\":\"10s\"}", str);
assertEquals("{\"translog\":{\"durability\":\"async\",\"flush_threshold_size\":\"256mb\"," +
"\"sync_interval\":\"10s\"}}", str);

IndexSettings deserialized = deserialize(str, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translogSyncInterval().time(), deserialized.translogSyncInterval().time());

var responseStr = "{\"translog\":{\"sync_interval\":\"10s\"}}";
IndexSettings responseDeserialized = deserialize(responseStr, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translogSyncInterval().time(), responseDeserialized.translog().syncInterval().time());
assertEquals(indexSettings.translog().syncInterval().time(), deserialized.translog().syncInterval().time());
assertEquals(indexSettings.translog().durability(), deserialized.translog().durability());
assertEquals(indexSettings.translog().flushThresholdSize(), deserialized.translog().flushThresholdSize());

var deprecatedForm = "{\"translog\":{\"sync_interval\":\"10s\"},\"translog.durability\":\"async\",\"translog" +
".flush_threshold_size\":\"256mb\"}";
IndexSettings deprecatedDeserialized = deserialize(deprecatedForm, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translog().syncInterval().time(), deprecatedDeserialized.translog().syncInterval().time());
assertEquals(indexSettings.translog().durability(), deprecatedDeserialized.translog().durability());
assertEquals(indexSettings.translog().flushThresholdSize(), deprecatedDeserialized.translog().flushThresholdSize());

} catch (JsonParsingException je) {
throw new JsonParsingException(je.getMessage() + " at " + je.getLocation(), je, je.getLocation());
Expand Down
21 changes: 11 additions & 10 deletions java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@

package org.opensearch.client.opensearch.integTest;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

import org.junit.Test;
import org.opensearch.Version;
import org.opensearch.client.opensearch.OpenSearchAsyncClient;
Expand Down Expand Up @@ -71,17 +79,10 @@
import org.opensearch.client.opensearch.indices.GetIndicesSettingsResponse;
import org.opensearch.client.opensearch.indices.GetMappingResponse;
import org.opensearch.client.opensearch.indices.IndexState;
import org.opensearch.client.opensearch.indices.Translog;
import org.opensearch.client.opensearch.model.ModelTestCase;
import org.opensearch.client.transport.endpoints.BooleanResponse;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

public abstract class AbstractRequestIT extends OpenSearchJavaClientTestCase {

@Test
Expand Down Expand Up @@ -118,7 +119,7 @@ public void testIndexSettings() throws Exception {
var createResponse = javaClient().indices()
.create(r -> r.index("test-settings")
.settings(s -> s
.translogSyncInterval(Time.of(t -> t.time("10s")))
.translog(Translog.of(tl -> tl.syncInterval(Time.of(t -> t.time("10s")))))
.mapping(m -> m
.fieldNameLength(f -> f.limit(300L))
.totalFields(f -> f.limit(30L))
Expand Down Expand Up @@ -153,7 +154,7 @@ public void testIndexSettings() throws Exception {
var putSettingsResponse = javaClient().indices().putSettings(r -> r
.index("test-settings")
.settings(s -> s
.translogSyncInterval(Time.of(t -> t.time("5s")))
.translog(Translog.of(tl -> tl.syncInterval(Time.of(t -> t.time("5s")))))
.mapping(m -> m
.fieldNameLength(f -> f.limit(400L))
.totalFields(f -> f.limit(130L))
Expand Down

0 comments on commit 8d06598

Please sign in to comment.