Merge branch 'improve-conf'
diff --git a/resources/adaptorlib/static/dashboard.js b/resources/adaptorlib/static/dashboard.js
index 1eec058..e3c9bd6 100755
--- a/resources/adaptorlib/static/dashboard.js
+++ b/resources/adaptorlib/static/dashboard.js
@@ -242,6 +242,27 @@
});
}
+function checkConfig() {
+ var sending = $('#gaf-check-config-sending');
+ sending.show();
+ rpc('checkForUpdatedConfig', null, function(result, error) {
+ sending.hide();
+ if (result === null) {
+ throw "Invalid response from server";
+ }
+ var notificationSpan = result ? $('#gaf-check-config-updated')
+ : $('#gaf-check-config-not-updated');
+ notificationSpan.show();
+ window.setTimeout(function() {
+ notificationSpan.fadeOut();
+ }, 5000);
+ if (result) {
+ // Config was updated; auto-update displayed config.
+ rpc('getConfig', null, getConfigCallback);
+ }
+ });
+}
+
function getLogCallback(result, error) {
if (result === null) {
throw error;
@@ -257,6 +278,7 @@
throw error;
} else {
var configTable = $('#gaf-config-table');
+ configTable.empty();
var keys = [];
var key;
for (key in result) {
@@ -327,4 +349,5 @@
rpc('getConfig', null, getConfigCallback);
rpc('getLog', null, getLogCallback);
$('#gaf-start-feed-push').click(startFeedPush);
+ $('#gaf-check-config').click(checkConfig);
});
diff --git a/resources/adaptorlib/static/index.html b/resources/adaptorlib/static/index.html
index 1723929..8454907 100755
--- a/resources/adaptorlib/static/index.html
+++ b/resources/adaptorlib/static/index.html
@@ -25,6 +25,13 @@
<span id="gaf-start-feed-push-error"></span>
</div>
+ <div>
+ <button id="gaf-check-config">Apply Updated Config</button>
+ <span id="gaf-check-config-sending" style="display: none">Sending request...</span>
+ <span id="gaf-check-config-updated" style="display: none">Configuration updated</span>
+ <span id="gaf-check-config-not-updated" style="display: none">No configuration changes noticed; nothing updated.</span>
+ </div>
+
<h2>Status</h2>
<table id="gaf-status-table"></table>
diff --git a/src/adaptorlib/Config.java b/src/adaptorlib/Config.java
index 2b2b7b1..fb6cc03 100644
--- a/src/adaptorlib/Config.java
+++ b/src/adaptorlib/Config.java
@@ -21,6 +21,7 @@
import java.nio.charset.Charset;
import java.text.MessageFormat;
import java.util.*;
+import java.util.concurrent.CopyOnWriteArrayList;
import java.util.logging.*;
/**
@@ -37,14 +38,19 @@
protected final Set<String> noDefaultConfig = new HashSet<String>();
/** Default configuration values. */
protected final Properties defaultConfig = new Properties();
- /** Overriding configuration values loaded from file. */
- protected Properties configFileProperties = new Properties(defaultConfig);
/** Overriding configuration values loaded from command line. */
- protected Properties config = new Properties(configFileProperties);
- protected File configFile = new File(DEFAULT_CONFIG_FILE);
+ // Reads require no additional locks, but modifications require lock on 'this'
+ // to prevent lost updates.
+ protected volatile Properties config = new Properties(defaultConfig);
+ /** Default configuration to use in {@link #loadDefaultConfigFile}. */
+ protected File defaultConfigFile = new File(DEFAULT_CONFIG_FILE);
+ /**
+ * The actual config file in use, or {@code null} if none have been loaded.
+ */
+ protected File configFile;
protected long configFileLastModified;
protected List<ConfigModificationListener> modificationListeners
- = new LinkedList<ConfigModificationListener>();
+ = new CopyOnWriteArrayList<ConfigModificationListener>();
public Config() {
String hostname = null;
@@ -79,8 +85,6 @@
addKey("adaptor.fullListingSchedule", "0 3 * * *");
// 15 minutes.
addKey("adaptor.incrementalPollPeriodSecs", "900");
- // In seconds.
- addKey("config.pollPeriodSecs", "30");
addKey("transform.pipeline", "");
// 1 MiB.
addKey("transform.maxDocumentBytes", "1048576");
@@ -302,13 +306,6 @@
}
/**
- * Period in milliseconds between checks for updated configuration.
- */
- public long getConfigPollPeriodMillis() {
- return Long.parseLong(getValue("config.pollPeriodSecs")) * 1000;
- }
-
- /**
* Returns a list of maps correspending to each transform in the pipeline.
* Each map is the configuration entries for that transform. The 'name'
* configuration entry is added in each map based on the name provided by the
@@ -391,76 +388,63 @@
}
/**
- * Load user-provided configuration file.
+ * Load user-provided configuration file, replacing any previously loaded file
+ * configuration.
*/
private void load(Reader configFile) throws IOException {
- configFileProperties.load(configFile);
+ Properties newConfigFileProperties = new Properties(defaultConfig);
+ newConfigFileProperties.load(configFile);
+
+ Config fakeOldConfig;
+ Set<String> differentKeys;
+ synchronized (this) {
+ // Create replacement config.
+ Properties newConfig = new Properties(newConfigFileProperties);
+ for (Object o : config.keySet()) {
+ newConfig.put(o, config.get(o));
+ }
+
+ // Find differences.
+ differentKeys = findDifferences(config, newConfig);
+
+ if (differentKeys.isEmpty()) {
+ log.info("No configuration changes found");
+ return;
+ }
+
+ validate(newConfig);
+
+ fakeOldConfig = new Config();
+ fakeOldConfig.config = config;
+ this.config = newConfig;
+ }
+ log.info("New configuration file loaded");
+ fireConfigModificationEvent(fakeOldConfig, differentKeys);
}
- Reader createReader(File file) throws IOException {
+ Reader createReader(File configFile) throws IOException {
return new InputStreamReader(new BufferedInputStream(
new FileInputStream(configFile)), Charset.forName("UTF-8"));
}
- public synchronized void ensureLatestConfigLoaded() throws IOException {
- if (!configFile.exists() || !configFile.isFile()) {
- return;
- }
- // Check for modifications.
- long newLastModified = configFile.lastModified();
- if (configFileLastModified == newLastModified || newLastModified == 0) {
- return;
- }
- log.info("Noticed modified configuration file");
- // Go ahead and update the modification time now, to prevent constantly
- // trying to load the configuration file, in case of errors.
- configFileLastModified = newLastModified;
-
- // Load freshly-modified file.
- Properties newConfigFileProperties = new Properties(defaultConfig);
- Properties newConfig = new Properties(newConfigFileProperties);
- Reader reader = createReader(configFile);
- try {
- newConfigFileProperties.load(reader);
- } finally {
- reader.close();
- }
-
- for (Object o : config.keySet()) {
- newConfig.put(o, config.get(o));
- }
-
- // Find differences.
- Set<String> differentKeys = findDifferences(config, newConfig);
-
- // Only allow adaptor.fullListingSchedule to be updated at the moment. No
- // other code can handle updates. Since the Dashboard will show the current
- // values, we don't want the Dashboard showing new values and the code using
- // old values. TODO(ejona): Once more things support modification of
- // configuration, this should be removed.
- for (String name : new ArrayList<String>(differentKeys)) {
- if (!"adaptor.fullListingSchedule".equals(name)) {
- differentKeys.remove(name);
- log.log(Level.INFO,
- "Ignoring modified key {0}, since it is not white-listed", name);
- newConfigFileProperties.setProperty(name, config.getProperty(name));
+ /**
+ * @return {@code true} if configuration file was modified.
+ */
+ public boolean ensureLatestConfigLoaded() throws IOException {
+ synchronized (this) {
+ if (configFile == null || !configFile.exists() || !configFile.isFile()) {
+ return false;
}
+ // Check for modifications.
+ long newLastModified = configFile.lastModified();
+ if (configFileLastModified == newLastModified || newLastModified == 0) {
+ return false;
+ }
+ log.info("Noticed modified configuration file");
+
+ load(configFile);
}
-
- if (differentKeys.isEmpty()) {
- log.info("No configuration changes found");
- return;
- }
-
- validate(newConfig);
-
- Config fakeOldConfig = new Config();
- fakeOldConfig.configFileProperties = configFileProperties;
- fakeOldConfig.config = config;
- this.configFileProperties = newConfigFileProperties;
- this.config = newConfig;
- log.info("New configuration file loaded");
- fireConfigModificationEvent(fakeOldConfig, differentKeys);
+ return true;
}
private Set<String> findDifferences(Properties config, Properties newConfig) {
@@ -486,6 +470,7 @@
* error handling, since this is typically non-fatal.
*/
public void loadDefaultConfigFile() {
+ configFile = defaultConfigFile;
if (configFile.exists() && configFile.isFile()) {
try {
load(configFile);
@@ -519,7 +504,6 @@
* @throws IllegalStateException when not all configuration keys have values
*/
public String[] autoConfig(String[] args) {
- loadDefaultConfigFile();
int i;
for (i = 0; i < args.length; i++) {
if (!args[i].startsWith("-D")) {
@@ -532,6 +516,7 @@
}
setValue(parts[0], parts[1]);
}
+ loadDefaultConfigFile();
validate();
if (i == 0) {
return args;
@@ -545,7 +530,7 @@
*
* @throws IllegalStateException if {@code key} has no value
*/
- public synchronized String getValue(String key) {
+ public String getValue(String key) {
String value = config.getProperty(key);
if (value == null) {
throw new IllegalStateException(MessageFormat.format(
@@ -612,30 +597,24 @@
public void addConfigModificationListener(
ConfigModificationListener listener) {
- synchronized (modificationListeners) {
- modificationListeners.add(listener);
- }
+ modificationListeners.add(listener);
}
public void removeConfigModificationListener(
ConfigModificationListener listener) {
- synchronized (modificationListeners) {
- modificationListeners.remove(listener);
- }
+ modificationListeners.remove(listener);
}
private void fireConfigModificationEvent(Config oldConfig,
Set<String> modifiedKeys) {
ConfigModificationEvent ev
= new ConfigModificationEvent(this, oldConfig, modifiedKeys);
- synchronized (modificationListeners) {
- for (ConfigModificationListener listener : modificationListeners) {
- try {
- listener.configModified(ev);
- } catch (Exception ex) {
- log.log(Level.WARNING,
- "Unexpected exception. Consider filing a bug.", ex);
- }
+ for (ConfigModificationListener listener : modificationListeners) {
+ try {
+ listener.configModified(ev);
+ } catch (Exception ex) {
+ log.log(Level.WARNING,
+ "Unexpected exception. Consider filing a bug.", ex);
}
}
}
diff --git a/src/adaptorlib/Dashboard.java b/src/adaptorlib/Dashboard.java
index 33d2eed..ffdd4c9 100644
--- a/src/adaptorlib/Dashboard.java
+++ b/src/adaptorlib/Dashboard.java
@@ -68,10 +68,12 @@
= new HttpsConfigurator(SSLContext.getDefault());
((HttpsServer) dashboardServer).setHttpsConfigurator(httpsConf);
}
- // If the port is zero, then the OS chose a port for us. This is mainly
- // useful during testing.
- dashboardPort = dashboardServer.getAddress().getPort();
- config.setValue("server.dashboardPort", "" + dashboardPort);
+ if (dashboardPort == 0) {
+ // If the port is zero, then the OS chose a port for us. This is mainly
+ // useful during testing.
+ dashboardPort = dashboardServer.getAddress().getPort();
+ config.setValue("server.dashboardPort", "" + dashboardPort);
+ }
// Use separate Executor for Dashboard to allow the administrator to
// investigate why things are going wrong without waiting on the normal work
// queue.
@@ -131,6 +133,8 @@
rpcHandler.registerRpcMethod("getConfig", new ConfigRpcMethod(config));
rpcHandler.registerRpcMethod("getStats", new StatRpcMethod(journal));
rpcHandler.registerRpcMethod("getStatuses", new StatusRpcMethod(monitor));
+ rpcHandler.registerRpcMethod("checkForUpdatedConfig",
+ new CheckForUpdatedConfigRpcMethod(gsaCommHandler));
return rpcHandler;
}
@@ -208,6 +212,19 @@
}
}
+ static class CheckForUpdatedConfigRpcMethod implements RpcHandler.RpcMethod {
+ private final GsaCommunicationHandler gsaComm;
+
+ public CheckForUpdatedConfigRpcMethod(GsaCommunicationHandler gsaComm) {
+ this.gsaComm = gsaComm;
+ }
+
+ @Override
+ public Object run(List request) {
+ return gsaComm.ensureLatestConfigLoaded();
+ }
+ }
+
static class LastPushStatusSource implements StatusSource {
private final Journal journal;
diff --git a/src/adaptorlib/GsaCommunicationHandler.java b/src/adaptorlib/GsaCommunicationHandler.java
index cef5b09..cfa90a4 100644
--- a/src/adaptorlib/GsaCommunicationHandler.java
+++ b/src/adaptorlib/GsaCommunicationHandler.java
@@ -54,14 +54,14 @@
* permits one invocation at a time. If multiple simultaneous invocations
* occur, all but the first will log a warning and return immediately.
*/
- private final OneAtATimeRunnable docIdFullPusher;
+ private final OneAtATimeRunnable docIdFullPusher = new OneAtATimeRunnable(
+ new PushRunnable(), new AlreadyRunningRunnable());
/**
* Schedule identifier for {@link #sendDocIds}.
*/
private String sendDocIdsSchedId;
private HttpServer server;
private Thread shutdownHook;
- private Timer configWatcherTimer = new Timer("configWatcher", true);
private IncrementalAdaptorPoller incrementalAdaptorPoller;
private final DocIdCodec docIdCodec;
private final DocIdSender docIdSender;
@@ -73,13 +73,10 @@
dashboard = new Dashboard(config, this, journal);
docIdCodec = new DocIdCodec(config);
- GsaFeedFileSender fileSender = new GsaFeedFileSender(
- config.getGsaCharacterEncoding(), config.isServerSecure());
+ GsaFeedFileSender fileSender = new GsaFeedFileSender(config);
GsaFeedFileMaker fileMaker = new GsaFeedFileMaker(docIdCodec);
docIdSender
= new DocIdSender(fileMaker, fileSender, journal, config, adaptor);
- docIdFullPusher = new OneAtATimeRunnable(
- new PushRunnable(), new AlreadyRunningRunnable());
}
/** Starts listening for communications from GSA. */
@@ -111,10 +108,12 @@
throw new RuntimeException(ex);
}
}
- // If the port is zero, then the OS chose a port for us. This is mainly
- // useful during testing.
- port = server.getAddress().getPort();
- config.setValue("server.port", "" + port);
+ if (port == 0) {
+ // If the port is zero, then the OS chose a port for us. This is mainly
+ // useful during testing.
+ port = server.getAddress().getPort();
+ config.setValue("server.port", "" + port);
+ }
int maxThreads = config.getServerMaxWorkerThreads();
int queueCapacity = config.getServerQueueCapacity();
BlockingQueue<Runnable> blockingQueue
@@ -168,7 +167,6 @@
Runtime.getRuntime().addShutdownHook(shutdownHook);
config.addConfigModificationListener(new GsaConfigModListener());
- TimerTask configWatcher = new ConfigWatcher(config);
long sleepDurationMillis = 1000;
// An hour.
@@ -182,7 +180,7 @@
Thread.sleep(sleepDurationMillis);
sleepDurationMillis
= Math.min(sleepDurationMillis * 2, maxSleepDurationMillis);
- configWatcher.run();
+ ensureLatestConfigLoaded();
}
}
@@ -203,9 +201,6 @@
scheduler.start();
sendDocIdsSchedId = scheduler.schedule(
config.getAdaptorFullListingSchedule(), docIdFullPusher);
-
- long period = config.getConfigPollPeriodMillis();
- configWatcherTimer.schedule(configWatcher, period, period);
}
private TransformPipeline createTransformPipeline() {
@@ -316,6 +311,16 @@
return docIdFullPusher.runInNewThread() != null;
}
+ boolean ensureLatestConfigLoaded() {
+ try {
+ return config.ensureLatestConfigLoaded();
+ } catch (Exception ex) {
+ log.log(Level.WARNING, "Error while trying to reload configuration",
+ ex);
+ return false;
+ }
+ }
+
/**
* Runnable that calls {@link DocIdSender#pushDocIds}.
*/
@@ -378,23 +383,25 @@
}
}
}
- }
- }
- private static class ConfigWatcher extends TimerTask {
- private Config config;
-
- public ConfigWatcher(Config config) {
- this.config = config;
- }
-
- @Override
- public void run() {
- try {
- config.ensureLatestConfigLoaded();
- } catch (Exception ex) {
- log.log(Level.WARNING, "Error while trying to reload configuration",
- ex);
+ // List of "safe" keys that can be updated without a restart.
+ List<String> safeKeys = Arrays.asList("adaptor.fullListingSchedule");
+ // Set of "unsafe" keys that have been modified.
+ Set<String> modifiedKeysRequiringRestart
+ = new HashSet<String>(modifiedKeys);
+ modifiedKeysRequiringRestart.removeAll(safeKeys);
+ // If there are modified "unsafe" keys, then we restart things to make
+ // sure all the code is up-to-date with the new values.
+ if (!modifiedKeysRequiringRestart.isEmpty()) {
+ log.warning("Unsafe configuration keys modified. To ensure a sane "
+ + "state, the adaptor is restarting.");
+ stop(3);
+ try {
+ start();
+ } catch (Exception ex) {
+ log.log(Level.SEVERE, "Automatic restart failed", ex);
+ throw new RuntimeException(ex);
+ }
}
}
}
diff --git a/src/adaptorlib/GsaFeedFileSender.java b/src/adaptorlib/GsaFeedFileSender.java
index e8b9772..19fd634 100644
--- a/src/adaptorlib/GsaFeedFileSender.java
+++ b/src/adaptorlib/GsaFeedFileSender.java
@@ -18,7 +18,6 @@
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
-import java.nio.charset.Charset;
import java.util.logging.Logger;
import java.util.zip.GZIPOutputStream;
@@ -50,11 +49,8 @@
// TODO(pjo): Add corrective tips.
}
- // All communications are expected to be tailored to GSA.
- private final Charset encoding;
-
- /** Whether to use HTTP or HTTPS to talk to the feedergate. */
- private final boolean useHttps;
+ /** Configuration for GSA's encoding and whether to use HTTPS. */
+ private final Config config;
// Feed file XML will not contain "<<".
private static final String BOUNDARY = "<<";
@@ -62,14 +58,13 @@
// Another frequently used constant of sent message.
private static final String CRLF = "\r\n";
- public GsaFeedFileSender(Charset encoding, boolean useHttps) {
- this.encoding = encoding;
- this.useHttps = useHttps;
+ public GsaFeedFileSender(Config config) {
+ this.config = config;
}
// Get bytes of string in communication's encoding.
private byte[] toEncodedBytes(String s) {
- return s.getBytes(encoding);
+ return s.getBytes(config.getGsaCharacterEncoding());
}
/** Helper method for creating a multipart/form-data HTTP post.
@@ -98,7 +93,7 @@
boolean useCompression)
throws MalformedURLException, IOException {
URL feedUrl;
- if (useHttps) {
+ if (config.isServerSecure()) {
feedUrl = new URL("https://" + gsaHost + ":19902/xmlfeed");
} else {
feedUrl = new URL("http://" + gsaHost + ":19900/xmlfeed");
@@ -136,7 +131,8 @@
BufferedReader br = null;
try {
InputStream inputStream = uc.getInputStream();
- br = new BufferedReader(new InputStreamReader(inputStream, encoding));
+ br = new BufferedReader(new InputStreamReader(
+ inputStream, config.getGsaCharacterEncoding()));
String line;
while ((line = br.readLine()) != null) {
buf.append(line);
diff --git a/test/adaptorlib/ConfigTest.java b/test/adaptorlib/ConfigTest.java
index 7801a41..cbc5dc6 100644
--- a/test/adaptorlib/ConfigTest.java
+++ b/test/adaptorlib/ConfigTest.java
@@ -149,6 +149,7 @@
+ "transform.pipeline.trans2.key2=value2\n"
+ "transform.pipeline.trans2.key3=value3\n"
);
+ config.setValue("gsa.hostname", "notreal");
config.load(configFile);
assertEquals(golden, config.getTransformPipelineSpec());
}
@@ -163,6 +164,7 @@
@Test
public void testGetTransformPipelineSpecInValid() throws Exception {
configFile.setFileContents("transform.pipeline=name1, ,name3\n");
+ config.setValue("gsa.hostname", "notreal");
config.load(configFile);
thrown.expect(RuntimeException.class);
config.getTransformPipelineSpec();
@@ -170,7 +172,7 @@
private static class ModifiedConfig extends Config {
public ModifiedConfig(MockFile file) {
- this.configFile = file;
+ this.defaultConfigFile = file;
}
@Override
diff --git a/test/adaptorlib/DocIdSenderTest.java b/test/adaptorlib/DocIdSenderTest.java
index df12e33..5142b12 100644
--- a/test/adaptorlib/DocIdSenderTest.java
+++ b/test/adaptorlib/DocIdSenderTest.java
@@ -240,7 +240,7 @@
List<String> xmlStrings = new ArrayList<String>();
public MockGsaFeedFileSender() {
- super(null, false);
+ super(new Config());
}
@Override
