eclipse-vertx / vert.x

Vert.x is a tool-kit for building reactive applications on the JVM
http://vertx.io
Other
14.31k stars 2.08k forks source link

FileResolver does not use vertx.disableFileCaching when using Eclipse #1514

Closed andrewwh closed 8 years ago

andrewwh commented 8 years ago

Problem: Setting the environment vertx.disableFileCaching to true does not disable file caching.

Cause: FileResolver statically initialises ENABLE_CACHING

Environment: Eclipse with dynamic reload: run main.WebVerticle --redeploy="src/*/.java" --launcher-class=io.vertx.core.Launcher -conf src/resources/conf/config.json -Dvertx.disableFileCaching=true

cescoffier commented 8 years ago

Can you explain what do you expect when disabling file caching ?

I've made a very simple app serving a single file over HTTP:

    public void start() throws Exception {
        vertx.createHttpServer().requestHandler(req -> {
          req.response().sendFile("test.txt");      
        }).listen(8080);
    }

I'm launching this app with the run configuration you gave above. The application starts, it serves the file. If I update the file, the updated file is served.

Do I miss something ?

andrewwh commented 8 years ago

Thanks for looking into this. I'll have to put together an example where it failed which I don't have at hand right now. I'll try and find the root cause and get an example soon.

Cheers, Andrew

On 2/08/2016 1:13 am, Clement Escoffier notifications@github.com wrote:

Can you explain what do you expect when disabling file caching ?

I've made a very simple app serving a single file over HTTP:

   @Override
public void start() throws Exception {
    vertx.createHttpServer().requestHandler(req -> {
      req.response().sendFile("test.txt");
    }).listen(8080);
}

I'm launching this app with the run configuration you gave above. The application starts, it serves the file. If I update the file, the updated file is served.

Do I miss something ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/eclipse/vert.x/issues/1514#issuecomment-236576595, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ATfHGmINwrL4ea51rBG5xc0HtTvny9k9ks5qbfEFgaJpZM4JLS1B.

andrewwh commented 8 years ago

This looks like a quirk of the JVM (sort of) but more of a bug with StartCommand. I did a bit more tracing of the launch process.

The eclipse launch command is: run volcanic.tephra.web.main.TephraWebVerticle --redeploy="src/*/.java" --launcher-class=io.vertx.core.Launcher -conf src/resources/conf/config.json -Dvertx.disableFileCaching=true

It is not possible to put the -Dvertx.disableFileCaching=true in the VM arguments for eclipse as this is only passed to the launcher and not the vert.x process (that is created by the StartCommand class). Should perhaps document this.

All the arguments are passed to the io.vertx.core.Launcher which in turn launches another java process. It is this second java process that needs the -D argument.

With a bit of tracing added, from the console: VertxCommandLauncher.dispatch : System Property vertx.disableFileCaching=false [When first launched the VM does not have this as a system property] Aug 03, 2016 8:07:42 PM io.vertx.core.impl.launcher.commands.Watcher INFO: Starting the vert.x application in redeploy mode Starting vert.x application... StartCommand.run : System Property vertx.disableFileCaching=true /usr/lib/jvm/java-8-oracle/jre/bin/java io.vertx.core.Launcher run volcanic.tephra.web.main.TephraWebVerticle --conf src/resources/conf/config.json -Dvertx.disableFileCaching=true -Dvertx.id=bc67f0b3-4f46-4cdf-940c-38baae45fa71-redeploy bc67f0b3-4f46-4cdf-940c-38baae45fa71-redeploy [Output from the StartCommand the JVM command created by ProcessBuilder before launching process] Launcher.main [The actual vert.x application] VertxCommandLauncher.dispatch : System Property vertx.disableFileCaching=false [The property is on the command line, but not available - maybe initialisation has not finished?] FileResolver.constructor : System Property vertx.disableFileCaching=true [When the FileResolver is constructed it is available, but would not be available for a static initializer]

So with the above java command that launches the actual vert.x instance the -D arguments are still being passed to the main class rather than to java as options. It may just be a quirk that the java process picks them up as java options. Normally java expects "java [options] [-jar jar] [arguments]".

I suspect that java processes them after static initialisation is complete (and maybe some other stuff ). I don't know that much about the process, so only a guess. This may also be a quirk of my jvm (I'm running java on Linux). It may be different on other platforms.

When I run the JVM command manually as:

/usr/lib/jvm/java-8-oracle/jre/bin/java -Dvertx.disableFileCaching=true io.vertx.core.Launcher run volcanic.tephra.web.main.TephraWebVerticle --conf src/resources/conf/config.json -Dvertx.id=bc67f0b3-4f46-4cdf-940c-38baae45fa71-redeploy bc67f0b3-4f46-4cdf-940c-38baae45fa71-redeploy

then the property is set to true in the VertxCommandLauncher.dispatch method.

So now that I think about it a bit more, perhaps the better fix is for the StartCommand.run method to reorganise the java arguments:

LinkedList<String> cmd = new LinkedList<>();
.
.
.
List<String> options = cmd.stream().filter(arg -> arg.startsWith("-D")).collect(Collectors.toList());
cmd.removeAll(options);
cmd.addAll(1, options);
cescoffier commented 8 years ago

When using the start command, parameter should be passed using: -java-opts="-D..."

I do agree that in the case of "-D" parameter the copy should be automatic. This should be fixed in the https://github.com/eclipse/vert.x/blob/master/src/main/java/io/vertx/core/impl/launcher/commands/RunCommand.java#L335 method.

So, it's actually a bug...

cescoffier commented 8 years ago

Hum, it should be done: https://github.com/eclipse/vert.x/blob/master/src/main/java/io/vertx/core/impl/launcher/commands/RunCommand.java#L375

andrewwh commented 8 years ago

One way to make this work is for vert.x to conform to the java convention of passing options before the main class. This means I can in Eclipse (as an example) set the following run configuration:

Main Class: io.vertx.core.Launcher Program arguments: run volcanic.tephra.web.main.TephraWebVerticle -conf src/resources/conf/config.json --redeploy="src/*/_.java" --launcher-class=io.vertx.core.Launcher _VM Arguments:** -Dvertx.disableFileCaching=true

You will notice that the JVM arguments are not passed in the program arguments.

Just to recap, the interesting boot sequence (some skipped) of events is: Launcher.main->VertxCommandLauncher.dispatch->RunCommand.run->RunCommand.initializeRedeployment->Watcher.watch->RunCommand.startAsBackgroundApplication->StartCommand.run->ProcessBuilder.start->Launch java

As this issue is probably limited to redeployment I suggest the following:

In the RunCommand, the initializeRedeployment needs to set any vert.x properties passed as JVM options. Ideally, I'd like to pass all user defined parameters, but I can't see anyway of determining this, I've limited it to vertx options:

  protected synchronized void initializeRedeployment() {
    if (watcher != null) {
      throw new IllegalStateException("Redeployment already started ? The watcher already exists");
    }

    // Need to copy any vert.x system properties from this JVM instance to the next
    System.getProperties().entrySet().stream()
                .filter(p -> p.getKey().toString().startsWith("vertx"))
                .forEach(p -> systemProperties.add(p.getKey() + "=" + p.getValue()));

The StartCommand needs to launch the java process with system options before the main class and arguments:

  public void run() {
    out.println("Starting vert.x application...");

    LinkedList<String> cmd = new LinkedList<>();
    ProcessBuilder builder = new ProcessBuilder();
    addJavaCommand(cmd);

    // Add system properties  which must be the first arguments to java
    if (systemProperties != null) {
      systemProperties.stream().map(entry -> "-D" + entry).forEach(arg -> ExecUtils.addArgument(cmd, arg));
    }
    // Add id - it's important as it's the application mark.
    cmd.add("-Dvertx.id=" + getId());

This also means getArguments() should not add system properties as they are not vert.x arguments.

The final background java process is launched as: /usr/lib/jvm/java-8-oracle/jre/bin/java -Dvertx.disableFileCaching=true -Dvertx.id=214b433f-7772-4ada-88c8-38e8d0f4abcb-redeploy io.vertx.core.Launcher run volcanic.tephra.web.main.TephraWebVerticle --conf src/resources/conf/config.json

With the options set as first arguments the system properties are available before static initialisation.

As I've only just started becoming familiar with the vert.x code base and given that this is a rather complex area I will defer to your better judgement.

cescoffier commented 8 years ago

I've tried to made a reproducer based on what you wrote. But it seems to work for me: https://github.com/cescoffier/vertx-1514-reproducer

Can you tell me what's different in your case ?

Vert.x read also system properties that are passed as verticle argument. And the FileResolver is correctly initialized with the passed values.

andrewwh commented 8 years ago

Unfortunately I can't reproduce this anymore!. I've recently upgraded my OS and JVM and now I don't get this problem. However, just to prove I'm not going insane I did a quick search and found an old stack overflow post with the same general problem:

http://stackoverflow.com/questions/5045608/proper-usage-of-java-d-command-line-parameters

I added a bit of code to my verticle:

    private void debug() {
        try {
            System.out.println("this.ENABLE_CACHING=" + !Boolean.getBoolean("vertx.disableFileCaching"));

            Field field = FileResolver.class.getDeclaredField("ENABLE_CACHING");
            field.setAccessible(true);
            System.out.println("FileResolver.ENABLE_CACHING=" + field.getBoolean(null));
        } catch (Exception e) {
            e.printStackTrace();
        }
    }

and it appears to pick up the JVM argument just fine. The order of the java arguments are still not correct according to the spec e.g. java [options] [jar | class] [program arguments], but I concede that it does work.

I would suggest that the documentation is changed however. When running eclipse with auto-redeploy using the Program arguments of --redeploy="src/*/.java" --launcher-class=io.vertx.core.Launcher then you cannot put the "-Dvertx.disableFileCaching=true" argument in the VM arguments field. It must be in the Program arguments, otherwise it is only applied to the launcher jvm not the jvm that actually runs the verticle.