Skip to content

Commit

Permalink
Merge branch '6.x' into silent-batch-mode-migration
Browse files Browse the repository at this point in the history
* 6.x:
  Move testMappingConflictRootCause to different class
  Enhance error for out of bounds byte size settings (elastic#29338)
  • Loading branch information
jasontedor committed Apr 4, 2018
2 parents 8c7fc3b + c570f09 commit cd3608a
Show file tree
Hide file tree
Showing 7 changed files with 99 additions and 60 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,17 +113,17 @@ public void testChunkSize() throws StorageException, IOException, URISyntaxExcep
// zero bytes is not allowed
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () ->
azureRepository(Settings.builder().put("chunk_size", "0").build()));
assertEquals("Failed to parse value [0] for setting [chunk_size] must be >= 1b", e.getMessage());
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getMessage());

// negative bytes not allowed
e = expectThrows(IllegalArgumentException.class, () ->
azureRepository(Settings.builder().put("chunk_size", "-1").build()));
assertEquals("Failed to parse value [-1] for setting [chunk_size] must be >= 1b", e.getMessage());
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getMessage());

// greater than max chunk size not allowed
e = expectThrows(IllegalArgumentException.class, () ->
azureRepository(Settings.builder().put("chunk_size", "65mb").build()));
assertEquals("Failed to parse value [65mb] for setting [chunk_size] must be <= 64mb", e.getMessage());
assertEquals("failed to parse value [65mb] for setting [chunk_size], must be <= [64mb]", e.getMessage());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -114,22 +114,22 @@ public void testChunkSize() {
Settings.builder().put("chunk_size", "0").build());
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData);
});
assertEquals("Failed to parse value [0] for setting [chunk_size] must be >= 1b", e.getMessage());
assertEquals("failed to parse value [0] for setting [chunk_size], must be >= [1b]", e.getMessage());

// negative bytes not allowed
e = expectThrows(IllegalArgumentException.class, () -> {
RepositoryMetaData repoMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE,
Settings.builder().put("chunk_size", "-1").build());
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData);
});
assertEquals("Failed to parse value [-1] for setting [chunk_size] must be >= 1b", e.getMessage());
assertEquals("failed to parse value [-1] for setting [chunk_size], must be >= [1b]", e.getMessage());

// greater than max chunk size not allowed
e = expectThrows(IllegalArgumentException.class, () -> {
RepositoryMetaData repoMetaData = new RepositoryMetaData("repo", GoogleCloudStorageRepository.TYPE,
Settings.builder().put("chunk_size", "101mb").build());
GoogleCloudStorageRepository.getSetting(GoogleCloudStorageRepository.CHUNK_SIZE, repoMetaData);
});
assertEquals("Failed to parse value [101mb] for setting [chunk_size] must be <= 100mb", e.getMessage());
assertEquals("failed to parse value [101mb] for setting [chunk_size], must be <= [100mb]", e.getMessage());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,10 @@ public void testInvalidChunkBufferSizeSettings() throws IOException {
assertValidBuffer(5, 5);
// buffer < 5mb should fail
assertInvalidBuffer(4, 10, IllegalArgumentException.class,
"Failed to parse value [4mb] for setting [buffer_size] must be >= 5mb");
"failed to parse value [4mb] for setting [buffer_size], must be >= [5mb]");
// chunk > 5tb should fail
assertInvalidBuffer(5, 6000000, IllegalArgumentException.class,
"Failed to parse value [6000000mb] for setting [chunk_size] must be <= 5tb");
"failed to parse value [6000000mb] for setting [chunk_size], must be <= [5tb]");
}

private void assertValidBuffer(long bufferMB, long chunkMB) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import java.util.IdentityHashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
Expand Down Expand Up @@ -1070,10 +1071,22 @@ public static Setting<ByteSizeValue> byteSizeSetting(String key, Function<Settin
public static ByteSizeValue parseByteSize(String s, ByteSizeValue minValue, ByteSizeValue maxValue, String key) {
ByteSizeValue value = ByteSizeValue.parseBytesSizeValue(s, key);
if (value.getBytes() < minValue.getBytes()) {
throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be >= " + minValue);
final String message = String.format(
Locale.ROOT,
"failed to parse value [%s] for setting [%s], must be >= [%s]",
s,
key,
minValue.getStringRep());
throw new IllegalArgumentException(message);
}
if (value.getBytes() > maxValue.getBytes()) {
throw new IllegalArgumentException("Failed to parse value [" + s + "] for setting [" + key + "] must be <= " + maxValue);
final String message = String.format(
Locale.ROOT,
"failed to parse value [%s] for setting [%s], must be <= [%s]",
s,
key,
maxValue.getStringRep());
throw new IllegalArgumentException(message);
}
return value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,6 @@
@ClusterScope(scope = Scope.TEST)
public class CreateIndexIT extends ESIntegTestCase {

@Override
protected Collection<Class<? extends Plugin>> nodePlugins() {
return Arrays.asList(InternalSettingsPlugin.class); // uses index.version.created
}

public void testCreationDateGivenFails() {
try {
prepareCreate("test").setSettings(Settings.builder().put(IndexMetaData.SETTING_CREATION_DATE, 4L)).get();
Expand Down Expand Up @@ -275,27 +270,6 @@ public void onFailure(Exception e) {
logger.info("total: {}", expected.getHits().getTotalHits());
}

/**
* Asserts that the root cause of mapping conflicts is readable.
*/
public void testMappingConflictRootCause() throws Exception {
CreateIndexRequestBuilder b = prepareCreate("test")
.setSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED,
VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_5_6_9)));
b.addMapping("type1", jsonBuilder().startObject().startObject("properties")
.startObject("text")
.field("type", "text")
.field("analyzer", "standard")
.field("search_analyzer", "whitespace")
.endObject().endObject().endObject());
b.addMapping("type2", jsonBuilder().humanReadable(true).startObject().startObject("properties")
.startObject("text")
.field("type", "text")
.endObject().endObject().endObject());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> b.get());
assertThat(e.getMessage(), containsString("mapper [text] is used by multiple types"));
}

public void testRestartIndexCreationAfterFullClusterRestart() throws Exception {
client().admin().cluster().prepareUpdateSettings().setTransientSettings(Settings.builder().put("cluster.routing.allocation.enable",
"none")).get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.unit.ByteSizeUnit;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.monitor.jvm.JvmInfo;
Expand Down Expand Up @@ -52,35 +53,61 @@ public void testGet() {
assertTrue(booleanSetting.get(Settings.builder().put("foo.bar", true).build()));
}

public void testByteSize() {
Setting<ByteSizeValue> byteSizeValueSetting =
Setting.byteSizeSetting("a.byte.size", new ByteSizeValue(1024), Property.Dynamic, Property.NodeScope);
public void testByteSizeSetting() {
final Setting<ByteSizeValue> byteSizeValueSetting =
Setting.byteSizeSetting("a.byte.size", new ByteSizeValue(1024), Property.Dynamic, Property.NodeScope);
assertFalse(byteSizeValueSetting.isGroupSetting());
ByteSizeValue byteSizeValue = byteSizeValueSetting.get(Settings.EMPTY);
assertEquals(byteSizeValue.getBytes(), 1024);

byteSizeValueSetting = Setting.byteSizeSetting("a.byte.size", s -> "2048b", Property.Dynamic, Property.NodeScope);
byteSizeValue = byteSizeValueSetting.get(Settings.EMPTY);
assertEquals(byteSizeValue.getBytes(), 2048);


final ByteSizeValue byteSizeValue = byteSizeValueSetting.get(Settings.EMPTY);
assertThat(byteSizeValue.getBytes(), equalTo(1024L));
}

public void testByteSizeSettingMinValue() {
final Setting<ByteSizeValue> byteSizeValueSetting =
Setting.byteSizeSetting(
"a.byte.size",
new ByteSizeValue(100, ByteSizeUnit.MB),
new ByteSizeValue(20_000_000, ByteSizeUnit.BYTES),
new ByteSizeValue(Integer.MAX_VALUE, ByteSizeUnit.BYTES));
final long value = 20_000_000 - randomIntBetween(1, 1024);
final Settings settings = Settings.builder().put("a.byte.size", value + "b").build();
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> byteSizeValueSetting.get(settings));
final String expectedMessage = "failed to parse value [" + value + "b] for setting [a.byte.size], must be >= [20000000b]";
assertThat(e, hasToString(containsString(expectedMessage)));
}

public void testByteSizeSettingMaxValue() {
final Setting<ByteSizeValue> byteSizeValueSetting =
Setting.byteSizeSetting(
"a.byte.size",
new ByteSizeValue(100, ByteSizeUnit.MB),
new ByteSizeValue(16, ByteSizeUnit.MB),
new ByteSizeValue(Integer.MAX_VALUE, ByteSizeUnit.BYTES));
final long value = (1L << 31) - 1 + randomIntBetween(1, 1024);
final Settings settings = Settings.builder().put("a.byte.size", value + "b").build();
final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> byteSizeValueSetting.get(settings));
final String expectedMessage = "failed to parse value [" + value + "b] for setting [a.byte.size], must be <= [2147483647b]";
assertThat(e, hasToString(containsString(expectedMessage)));
}

public void testByteSizeSettingValidation() {
final Setting<ByteSizeValue> byteSizeValueSetting =
Setting.byteSizeSetting("a.byte.size", s -> "2048b", Property.Dynamic, Property.NodeScope);
final ByteSizeValue byteSizeValue = byteSizeValueSetting.get(Settings.EMPTY);
assertThat(byteSizeValue.getBytes(), equalTo(2048L));
AtomicReference<ByteSizeValue> value = new AtomicReference<>(null);
ClusterSettings.SettingUpdater<ByteSizeValue> settingUpdater = byteSizeValueSetting.newUpdater(value::set, logger);
try {
settingUpdater.apply(Settings.builder().put("a.byte.size", 12).build(), Settings.EMPTY);
fail("no unit");
} catch (IllegalArgumentException ex) {
assertThat(ex, hasToString(containsString("illegal value can't update [a.byte.size] from [2048b] to [12]")));
assertNotNull(ex.getCause());
assertThat(ex.getCause(), instanceOf(IllegalArgumentException.class));
final IllegalArgumentException cause = (IllegalArgumentException) ex.getCause();
final String expected =
"failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized";
assertThat(cause, hasToString(containsString(expected)));
}

final IllegalArgumentException e = expectThrows(
IllegalArgumentException.class,
() -> settingUpdater.apply(Settings.builder().put("a.byte.size", 12).build(), Settings.EMPTY));
assertThat(e, hasToString(containsString("illegal value can't update [a.byte.size] from [2048b] to [12]")));
assertNotNull(e.getCause());
assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
final IllegalArgumentException cause = (IllegalArgumentException) e.getCause();
final String expected = "failed to parse setting [a.byte.size] with value [12] as a size in bytes: unit is missing or unrecognized";
assertThat(cause, hasToString(containsString(expected)));
assertTrue(settingUpdater.apply(Settings.builder().put("a.byte.size", "12b").build(), Settings.EMPTY));
assertEquals(new ByteSizeValue(12), value.get());
assertThat(value.get(), equalTo(new ByteSizeValue(12)));
}

public void testMemorySize() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@
package org.elasticsearch.indices.mapping;

import org.elasticsearch.Version;
import org.elasticsearch.action.admin.indices.create.CreateIndexRequestBuilder;
import org.elasticsearch.action.admin.indices.mapping.get.GetMappingsResponse;
import org.elasticsearch.action.admin.indices.mapping.put.PutMappingResponse;
import org.elasticsearch.action.admin.indices.refresh.RefreshResponse;
import org.elasticsearch.action.index.IndexRequestBuilder;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.metadata.MappingMetaData;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.collect.ImmutableOpenMap;
Expand All @@ -37,6 +39,7 @@
import org.elasticsearch.plugins.Plugin;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalSettingsPlugin;
import org.elasticsearch.test.VersionUtils;
import org.hamcrest.Matchers;

import java.util.ArrayList;
Expand All @@ -53,6 +56,7 @@
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_BLOCKS_READ;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_BLOCKS_WRITE;
import static org.elasticsearch.cluster.metadata.IndexMetaData.SETTING_READ_ONLY;
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertBlocked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertThrows;
Expand All @@ -69,6 +73,27 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
return Collections.singleton(InternalSettingsPlugin.class);
}

/**
* Asserts that the root cause of mapping conflicts is readable.
*/
public void testMappingConflictRootCause() throws Exception {
CreateIndexRequestBuilder b = prepareCreate("test")
.setSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED,
VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, Version.V_5_6_9)));
b.addMapping("type1", jsonBuilder().startObject().startObject("properties")
.startObject("text")
.field("type", "text")
.field("analyzer", "standard")
.field("search_analyzer", "whitespace")
.endObject().endObject().endObject());
b.addMapping("type2", jsonBuilder().humanReadable(true).startObject().startObject("properties")
.startObject("text")
.field("type", "text")
.endObject().endObject().endObject());
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> b.get());
assertThat(e.getMessage(), containsString("mapper [text] is used by multiple types"));
}

public void testDynamicUpdates() throws Exception {
client().admin().indices().prepareCreate("test")
.setSettings(
Expand Down

0 comments on commit cd3608a

Please sign in to comment.