launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

Double reload with local file and autoUpdate=true #138

Closed scinos closed 5 years ago

scinos commented 5 years ago

When reading FeatureFlags form a local file and using autoUpdate, it seems to reload it multiple times.

After saving the file once (with no actual changes on the content of the file), I got

dev_1          | [2019-02-15T01:36:18.712Z]  WARN: ins/41 on ad5a8c3d9e47: Reloaded flags from file data (timestamp=2019-02-15T01:36:18.712Z)
dev_1          | [2019-02-15T01:36:18.715Z]  WARN: ins/41 on ad5a8c3d9e47: Reloaded flags from file data (timestamp=2019-02-15T01:36:18.715Z)

Some times I even get 3 or 4 reloads.

Relevant code:

const config = {
    logger,
};

if (LAUNCHDARKLY_SDK === 'local') {
    config.sendEvents = false;
    config.updateProcessor = LaunchDarkly.FileDataSource({ paths: [localOverridePath], autoUpdate: true });
}

ldClient = LaunchDarkly.init(LAUNCHDARKLY_SDK, config);
return ldClient.waitForInitialization();

Versions:

ldclient-node version 5.7.1
Node version 10.15.1 running inside a Docker container
Docker version 18.09.1, build 4c52b90
OSX version 10.14.3 (18D109)
eli-darkly commented 5 years ago

Thanks for reporting this. We'll see if we can reproduce that behavior.

There are two other pieces of information that might be helpful:

  1. What's the OS inside the Docker container?
  2. Where is localOverridePath located relative to the application? Like, is it in the same directory as your source code, or is it in a special location like /tmp, etc.

Also, do I understand correctly that it starts out just loading the file at startup time, and doesn't do any unexpected reloads, until you do modify the file (or at least touch it in a way that modifies its timestamp), and then you get a bunch of reloads after that? And do the reloads always happen very close to each other?

eli-darkly commented 5 years ago

Yeah, this wasn't hard to reproduce - I'm surprised we didn't see it earlier. It seems there's a known issue where Node's fs.watch(), which is what this uses, can report extra change events for some reason. There are some other packages available that claim to be able to fix this, but rather than add more dependencies to the SDK, I think we'll probably just add a check on the file's last-modified timestamp.

scinos commented 5 years ago

In case it still helps:

eli-darkly commented 5 years ago

Thanks. The fix is indeed pretty easy for Linux and Mac, but I'm still seeing some weird behavior on Windows (which we just started testing with recently), so I'll put out a patch as soon as I've figured that out.

eli-darkly commented 5 years ago

We've released version 5.7.2 with a fix for this.