apply review comments

This commit is contained in:
Simon Willnauer 2015-10-22 23:06:59 +02:00
parent 2245f480c9
commit f8248eda61
5 changed files with 41 additions and 18 deletions

View File

@ -131,7 +131,7 @@ public class TransportNodesListGatewayStartedShards extends TransportNodesAction
if (metaData != null) {
ShardPath shardPath = null;
try {
IndexSettings indexSettings = new IndexSettings(new Index(metaData.getIndex()), Settings.settingsBuilder().put(settings).put(metaData.getSettings()).build(), Collections.EMPTY_LIST);
IndexSettings indexSettings = new IndexSettings(shardId.index(), Settings.settingsBuilder().put(settings).put(metaData.getSettings()).build(), Collections.EMPTY_LIST);
shardPath = ShardPath.loadShardPath(logger, nodeEnv, shardId, indexSettings);
if (shardPath == null) {
throw new IllegalStateException(shardId + " no shard path found");

View File

@ -90,7 +90,7 @@ public class IndexModule extends AbstractModule {
* Note: an index might be created on a node multiple times. For instance if the last shard from an index is
* relocated to another node the internal representation will be destroyed which includes the registered listeners.
* Once the node holds at least one shard of an index all modules are reloaded and listeners are registered again.
* Listeners can't be unregistered the will stay alive for the entire time the index is allocated on a node.
* Listeners can't be unregistered they will stay alive for the entire time the index is allocated on a node.
* </p>
*/
public void addIndexEventListener(IndexEventListener listener) {

View File

@ -57,7 +57,6 @@ import java.nio.file.Path;
import java.util.*;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
@ -509,7 +508,7 @@ public class IndexService extends AbstractIndexComponent implements IndexCompone
public synchronized void updateMetaData(final IndexMetaData metadata) {
this.indexMetaData = metadata;
Settings settings = metadata.getSettings();
if (this.indexSettings.updateSettings(metadata.getSettings())) {
if (this.indexSettings.updateIndexSettings(metadata.getSettings())) {
for (final IndexShard shard : this.shards.values()) {
try {
shard.onRefreshSettings(settings);

View File

@ -112,25 +112,26 @@ public final class IndexSettings {
*
* @return <code>true</code> iff any setting has been updated otherwise <code>false</code>.
*/
synchronized boolean updateSettings(Settings settings) {
if (Version.indexCreated(settings) != version) {
throw new IllegalArgumentException("version mismatch on settings update expected: " + version + " but was: " + Version.indexCreated(settings));
synchronized boolean updateIndexSettings(Settings newSettings) {
if (Version.indexCreated(newSettings) != version) {
throw new IllegalArgumentException("version mismatch on settings update expected: " + version + " but was: " + Version.indexCreated(newSettings));
}
final String newUUID = settings.get(IndexMetaData.SETTING_INDEX_UUID, IndexMetaData.INDEX_UUID_NA_VALUE);
final String newUUID = newSettings.get(IndexMetaData.SETTING_INDEX_UUID, IndexMetaData.INDEX_UUID_NA_VALUE);
if (newUUID.equals(getUUID()) == false) {
throw new IllegalArgumentException("uuid mismatch on settings update expected: " + uuid + " but was: " + newUUID);
}
if (this.settings.getByPrefix(IndexMetaData.INDEX_SETTING_PREFIX).getAsMap().equals(settings.getByPrefix(IndexMetaData.INDEX_SETTING_PREFIX).getAsMap())) {
final Settings existingSettings = this.settings;
if (existingSettings.getByPrefix(IndexMetaData.INDEX_SETTING_PREFIX).getAsMap().equals(newSettings.getByPrefix(IndexMetaData.INDEX_SETTING_PREFIX).getAsMap())) {
// nothing to update, same settings
return false;
}
this.settings = Settings.builder().put(this.settings).put(settings).build();
this.settings = Settings.builder().put(existingSettings).put(newSettings).build();
final Settings mergedSettings = this.settings;
for (final Consumer<Settings> consumer : updateListeners) {
try {
consumer.accept(settings);
consumer.accept(mergedSettings);
} catch (Exception e) {
logger.warn("failed to refresh index settings for [{}]", e, settings);
logger.warn("failed to refresh index settings for [{}]", e, mergedSettings);
}
}
return true;

View File

@ -42,13 +42,36 @@ public class IndexSettingsTests extends ESTestCase {
assertEquals("0xdeadbeef", settings.getUUID());
assertEquals(1, settings.getUpdateListeners().size());
assertFalse(settings.updateSettings(theSettings));
assertFalse(settings.updateIndexSettings(theSettings));
assertSame(theSettings, settings.getSettings());
assertEquals(0, integer.get());
assertTrue(settings.updateSettings(Settings.builder().put(theSettings).put("index.test.setting.int", 42).build()));
assertTrue(settings.updateIndexSettings(Settings.builder().put(theSettings).put("index.test.setting.int", 42).build()));
assertEquals(42, integer.get());
}
public void testMergedSettingsArePassed() {
Version version = VersionUtils.getPreviousVersion();
Settings theSettings = Settings.settingsBuilder().put(IndexMetaData.SETTING_VERSION_CREATED, version)
.put(IndexMetaData.SETTING_INDEX_UUID, "0xdeadbeef").build();
final AtomicInteger integer = new AtomicInteger(0);
final StringBuilder builder = new StringBuilder();
Consumer<Settings> settingsConsumer = (s) -> {
integer.set(s.getAsInt("index.test.setting.int", -1));
builder.append(s.get("not.updated", ""));
};
IndexSettings settings = new IndexSettings(new Index("index"), theSettings, Collections.singleton(settingsConsumer));
assertEquals(0, integer.get());
assertEquals("", builder.toString());
assertTrue(settings.updateIndexSettings(Settings.builder().put(theSettings).put("index.test.setting.int", 42).build()));
assertEquals(42, integer.get());
assertEquals("", builder.toString());
integer.set(0);
assertTrue(settings.updateIndexSettings(Settings.builder().put(theSettings).put("not.updated", "boom").build()));
assertEquals("boom", builder.toString());
assertEquals(42, integer.get());
}
public void testListenerCanThrowException() {
Version version = VersionUtils.getPreviousVersion();
Settings theSettings = Settings.settingsBuilder().put(IndexMetaData.SETTING_VERSION_CREATED, version).put(IndexMetaData.SETTING_INDEX_UUID, "0xdeadbeef").build();
@ -61,7 +84,7 @@ public class IndexSettingsTests extends ESTestCase {
Collections.shuffle(list, random());
IndexSettings settings = new IndexSettings(new Index("index"), theSettings, list);
assertEquals(0, integer.get());
assertTrue(settings.updateSettings(Settings.builder().put(theSettings).put("index.test.setting.int", 42).build()));
assertTrue(settings.updateIndexSettings(Settings.builder().put(theSettings).put("index.test.setting.int", 42).build()));
assertEquals(42, integer.get());
}
@ -72,7 +95,7 @@ public class IndexSettingsTests extends ESTestCase {
assertEquals(version, settings.getIndexVersionCreated());
assertEquals("_na_", settings.getUUID());
try {
settings.updateSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).put("index.test.setting.int", 42).build());
settings.updateIndexSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).put("index.test.setting.int", 42).build());
fail("version has changed");
} catch (IllegalArgumentException ex) {
assertTrue(ex.getMessage(), ex.getMessage().startsWith("version mismatch on settings update expected: "));
@ -81,7 +104,7 @@ public class IndexSettingsTests extends ESTestCase {
theSettings = Settings.settingsBuilder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).put(IndexMetaData.SETTING_INDEX_UUID, "0xdeadbeef").build();
settings = new IndexSettings(new Index("index"), theSettings, Collections.EMPTY_LIST);
try {
settings.updateSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).put("index.test.setting.int", 42).build());
settings.updateIndexSettings(Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT).put("index.test.setting.int", 42).build());
fail("uuid missing/change");
} catch (IllegalArgumentException ex) {
assertEquals("uuid mismatch on settings update expected: 0xdeadbeef but was: _na_", ex.getMessage());