Ensure we have a consistent view on OperationMode in Watcher (elastic/elasticsearch#3507)
Today the operation mode can be set to default for a short amout of time until it's reset to the actual mode this can cause weird sideeffects for users if it's read concurrently. Also the test relies on a certain happens before relationship that is not guaranteed since the operation mode is set before the listerner is run. This change also rewrites the test to not use busy waiting but wait for the actual listern to be executed. Original commit: elastic/x-pack-elasticsearch@a2a42b89e5
This commit is contained in:
parent
c21a922778
commit
7be765d2a0
|
@ -9,6 +9,8 @@ package org.elasticsearch.license;
|
|||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.logging.log4j.message.ParameterizedMessage;
|
||||
import org.apache.logging.log4j.util.Supplier;
|
||||
import org.apache.lucene.util.BytesRef;
|
||||
import org.elasticsearch.common.bytes.BytesReference;
|
||||
import org.elasticsearch.license.License.OperationMode;
|
||||
import org.elasticsearch.watcher.FileChangesListener;
|
||||
import org.elasticsearch.watcher.FileWatcher;
|
||||
|
@ -90,27 +92,34 @@ public final class OperationModeFileWatcher implements FileChangesListener {
|
|||
|
||||
private synchronized void onChange(Path file) {
|
||||
if (file.equals(licenseModePath)) {
|
||||
currentOperationMode = defaultOperationMode;
|
||||
if (Files.exists(licenseModePath)
|
||||
&& Files.isReadable(licenseModePath)) {
|
||||
final byte[] content;
|
||||
try {
|
||||
content = Files.readAllBytes(licenseModePath);
|
||||
} catch (IOException e) {
|
||||
logger.error(
|
||||
(Supplier<?>) () -> new ParameterizedMessage(
|
||||
"couldn't read operation mode from [{}]", licenseModePath.toAbsolutePath()), e);
|
||||
return;
|
||||
}
|
||||
String operationMode = new String(content, StandardCharsets.UTF_8);
|
||||
try {
|
||||
currentOperationMode = OperationMode.resolve(operationMode);
|
||||
} catch (IllegalArgumentException e) {
|
||||
logger.error(
|
||||
(Supplier<?>) () -> new ParameterizedMessage(
|
||||
"invalid operation mode in [{}]", licenseModePath.toAbsolutePath()), e);
|
||||
return;
|
||||
OperationMode newOperationMode = defaultOperationMode;
|
||||
try {
|
||||
if (Files.exists(licenseModePath)
|
||||
&& Files.isReadable(licenseModePath)) {
|
||||
final byte[] content;
|
||||
try {
|
||||
content = Files.readAllBytes(licenseModePath);
|
||||
} catch (IOException e) {
|
||||
logger.error(
|
||||
(Supplier<?>) () -> new ParameterizedMessage(
|
||||
"couldn't read operation mode from [{}]", licenseModePath.toAbsolutePath()), e);
|
||||
return;
|
||||
}
|
||||
// this UTF-8 conversion is much pickier than java String
|
||||
final String operationMode = new BytesRef(content).utf8ToString();
|
||||
try {
|
||||
newOperationMode = OperationMode.resolve(operationMode);
|
||||
} catch (IllegalArgumentException e) {
|
||||
logger.error(
|
||||
(Supplier<?>) () -> new ParameterizedMessage(
|
||||
"invalid operation mode in [{}]", licenseModePath.toAbsolutePath()), e);
|
||||
return;
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
// set this after the fact to prevent that we are jumping back and forth first setting to defautl and then reading the
|
||||
// actual op mode resetting it.
|
||||
this.currentOperationMode = newOperationMode;
|
||||
}
|
||||
onChange.run();
|
||||
}
|
||||
|
|
|
@ -17,7 +17,9 @@ import java.nio.charset.StandardCharsets;
|
|||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.StandardOpenOption;
|
||||
import java.util.concurrent.atomic.AtomicInteger;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
|
||||
|
@ -26,7 +28,7 @@ public class OperationModeFileWatcherTests extends ESTestCase {
|
|||
private TestThreadPool threadPool;
|
||||
private Path licenseModePath;
|
||||
private OperationModeFileWatcher operationModeFileWatcher;
|
||||
private AtomicInteger onChangeCounter;
|
||||
private AtomicReference<CountDownLatch> onChangeCounter;
|
||||
|
||||
@Before
|
||||
public void setup() throws Exception {
|
||||
|
@ -38,9 +40,9 @@ public class OperationModeFileWatcherTests extends ESTestCase {
|
|||
threadPool);
|
||||
watcherService.start();
|
||||
licenseModePath = createTempFile();
|
||||
onChangeCounter = new AtomicInteger();
|
||||
onChangeCounter = new AtomicReference<>(new CountDownLatch(1));
|
||||
operationModeFileWatcher = new OperationModeFileWatcher(watcherService, licenseModePath, logger,
|
||||
() -> onChangeCounter.incrementAndGet());
|
||||
() -> onChangeCounter.get().countDown());
|
||||
}
|
||||
|
||||
@After
|
||||
|
@ -50,10 +52,11 @@ public class OperationModeFileWatcherTests extends ESTestCase {
|
|||
}
|
||||
|
||||
public void testInit() throws Exception {
|
||||
onChangeCounter.set(new CountDownLatch(2));
|
||||
writeMode("gold");
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.PLATINUM));
|
||||
operationModeFileWatcher.init();
|
||||
assertThat(onChangeCounter.get(), equalTo(2));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD));
|
||||
}
|
||||
|
||||
|
@ -62,20 +65,25 @@ public class OperationModeFileWatcherTests extends ESTestCase {
|
|||
operationModeFileWatcher.init();
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.PLATINUM));
|
||||
writeMode("gold");
|
||||
assertBusy(() -> assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD)));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD));
|
||||
onChangeCounter.set(new CountDownLatch(1));
|
||||
writeMode("basic");
|
||||
assertBusy(() -> assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.BASIC)));
|
||||
assertThat(onChangeCounter.get(), equalTo(2));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.BASIC));
|
||||
}
|
||||
|
||||
public void testDeleteModeFromFile() throws Exception {
|
||||
Files.delete(licenseModePath);
|
||||
operationModeFileWatcher.init();
|
||||
writeMode("gold");
|
||||
assertBusy(() -> assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD)));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD));
|
||||
onChangeCounter.set(new CountDownLatch(1));
|
||||
|
||||
Files.delete(licenseModePath);
|
||||
assertBusy(() -> assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.PLATINUM)));
|
||||
assertThat(onChangeCounter.get(), equalTo(2));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.PLATINUM));
|
||||
}
|
||||
|
||||
public void testInvalidModeFromFile() throws Exception {
|
||||
|
@ -98,10 +106,12 @@ public class OperationModeFileWatcherTests extends ESTestCase {
|
|||
Files.delete(licenseModePath);
|
||||
operationModeFileWatcher.init();
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.PLATINUM));
|
||||
onChangeCounter.set(new CountDownLatch(1));
|
||||
Path tempFile = createTempFile();
|
||||
writeMode("gold", tempFile);
|
||||
licenseModePath = tempFile;
|
||||
assertBusy(() -> assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD)));
|
||||
assertTrue(onChangeCounter.get().await(5, TimeUnit.SECONDS));
|
||||
assertThat(operationModeFileWatcher.getCurrentOperationMode(), equalTo(License.OperationMode.GOLD));
|
||||
}
|
||||
|
||||
private void writeMode(String mode) throws IOException {
|
||||
|
|
Loading…
Reference in New Issue