allegro / embedded-elasticsearch

Tool that ease up creation of integration tests with Elasticsearch
Apache License 2.0
269 stars 81 forks source link

idempotent start #64

Closed amir-arad closed 5 years ago

amir-arad commented 6 years ago

use case: I'm using embedded-elasticsearch for tests, where some of the tests turn elasticsearch off to simulate errors, so that between any two tests i want to validate that the elasticsearch is back on again. for that I use a reset method:

  def reset() = {
    embeddedElastic.start // theoretically idempotent
    embeddedElastic.deleteIndices
    embeddedElastic.createIndices
  }

I assume that the start() method in EmbeddedElastic is supposedly idempotent as far as the internal ElasticServer instance is concerned: because I see that it's only calling the .start() method if it receives false on isStarted() :

    private void startElastic() throws IOException, InterruptedException {
        if (!elasticServer.isStarted()) {
            elasticServer.start();
        }
    }

however, isStarted() is implemented by optimistically, lock-free reading the started flag:

    boolean isStarted() {
        return started;
    }

and the started flag may have already been written to by another thread listening to the logs in signalElasticStarted():

    private void signalElasticStarted() {
        synchronized (startedLock) {
            started = true;
            startedLock.notifyAll();
        }
    }

I'm solving this locally by wrapping the EmbeddedElastic with a proxy that has idempotent ensureStarted() and ensureStopped() by means of its own started flag. I think this is a feature that is should move into EmbeddedElastic itself so that other users may enjoy mindlessly resetting the state of the server with no need to track who did what to it beforehand.

gaczm commented 6 years ago

Hi :) Given that you have solution for this, can you submit pull request?