From 01a070c9b71cc7044c4f89fd333738db64719f9d Mon Sep 17 00:00:00 2001 From: RockyMandayam2 <106835690+RockyMandayam2@users.noreply.github.com> Date: Wed, 28 Sep 2022 12:37:19 -0700 Subject: [PATCH] Only push updates for desired zones/venues (#80) --- .../facebook/openwifirrm/RRMAlgorithm.java | 16 ++++- .../openwifirrm/modules/ApiServer.java | 23 ++++--- .../openwifirrm/modules/ConfigManager.java | 63 +++++++++++++++---- .../openwifirrm/modules/RRMScheduler.java | 8 ++- .../optimizers/channel/ChannelOptimizer.java | 8 +-- .../openwifirrm/optimizers/tpc/TPC.java | 8 +-- 6 files changed, 90 insertions(+), 36 deletions(-) diff --git a/src/main/java/com/facebook/openwifirrm/RRMAlgorithm.java b/src/main/java/com/facebook/openwifirrm/RRMAlgorithm.java index f551cd6..75e5c5b 100644 --- a/src/main/java/com/facebook/openwifirrm/RRMAlgorithm.java +++ b/src/main/java/com/facebook/openwifirrm/RRMAlgorithm.java @@ -143,6 +143,9 @@ public class RRMAlgorithm { * @param dryRun if set, do not apply changes * @param allowDefaultMode if false, "mode" argument must be present and * valid (returns error if invalid) + * @param updateImmediately true if the method should queue the zone for + * update and interrupt the config manager thread + * to trigger immediate update * * @return the algorithm result, with exactly one field set ("error" upon * failure, any others upon success) @@ -153,7 +156,8 @@ public class RRMAlgorithm { Modeler modeler, String zone, boolean dryRun, - boolean allowDefaultMode + boolean allowDefaultMode, + boolean updateImmediately ) { AlgorithmResult result = new AlgorithmResult(); if (name == null || args == null) { @@ -212,11 +216,14 @@ public class RRMAlgorithm { } result.channelMap = optimizer.computeChannelMap(); if (!dryRun) { - optimizer.applyConfig( + optimizer.updateDeviceApConfig( deviceDataManager, configManager, result.channelMap ); + if (updateImmediately) { + configManager.queueZoneAndWakeUp(zone); + } } } else if ( name.equals(RRMAlgorithm.AlgorithmType.OptimizeTxPower.name()) @@ -270,11 +277,14 @@ public class RRMAlgorithm { } result.txPowerMap = optimizer.computeTxPowerMap(); if (!dryRun) { - optimizer.applyConfig( + optimizer.updateDeviceApConfig( deviceDataManager, configManager, result.txPowerMap ); + if (updateImmediately) { + configManager.queueZoneAndWakeUp(zone); + } } } else { result.error = String.format("Unknown algorithm: '%s'", name); diff --git a/src/main/java/com/facebook/openwifirrm/modules/ApiServer.java b/src/main/java/com/facebook/openwifirrm/modules/ApiServer.java index 10d4c32..8fabab3 100644 --- a/src/main/java/com/facebook/openwifirrm/modules/ApiServer.java +++ b/src/main/java/com/facebook/openwifirrm/modules/ApiServer.java @@ -712,7 +712,8 @@ public class ApiServer implements Runnable { modeler, venue, mock, - true /* allowDefaultMode */ + true, /* allowDefaultMode */ + true /* updateImmediately */ ); if (result.error != null) { response.status(400); @@ -918,7 +919,7 @@ public class ApiServer implements Runnable { DeviceConfig networkConfig = gson.fromJson(request.body(), DeviceConfig.class); deviceDataManager.setDeviceNetworkConfig(networkConfig); - configManager.wakeUp(); + configManager.queueAllZonesAndWakeUp(); // Revalidate data model modeler.revalidate(); @@ -982,7 +983,7 @@ public class ApiServer implements Runnable { DeviceConfig zoneConfig = gson.fromJson(request.body(), DeviceConfig.class); deviceDataManager.setDeviceZoneConfig(zone, zoneConfig); - configManager.wakeUp(); + configManager.queueZoneAndWakeUp(zone); // Revalidate data model modeler.revalidate(); @@ -1045,7 +1046,10 @@ public class ApiServer implements Runnable { DeviceConfig apConfig = gson.fromJson(request.body(), DeviceConfig.class); deviceDataManager.setDeviceApConfig(serialNumber, apConfig); - configManager.wakeUp(); + // TODO enable updates to device(s), not just the entire zone + final String zone = + deviceDataManager.getDeviceZone(serialNumber); + configManager.queueZoneAndWakeUp(zone); // Revalidate data model modeler.revalidate(); @@ -1118,7 +1122,10 @@ public class ApiServer implements Runnable { .computeIfAbsent(serialNumber, k -> new DeviceConfig()) .apply(apConfig); }); - configManager.wakeUp(); + final String zone = + deviceDataManager.getDeviceZone(serialNumber); + // TODO enable updates to device(s), not just the entire zone + configManager.queueZoneAndWakeUp(zone); // Revalidate data model modeler.revalidate(); @@ -1261,7 +1268,8 @@ public class ApiServer implements Runnable { modeler, zone, dryRun, - false /* allowDefaultMode */ + false, /* allowDefaultMode */ + true /* updateImmediately */ ); if (result.error != null) { response.status(400); @@ -1372,7 +1380,8 @@ public class ApiServer implements Runnable { modeler, zone, dryRun, - false /* allowDefaultMode */ + false, /* allowDefaultMode */ + true /* updateImmediately */ ); if (result.error != null) { response.status(400); diff --git a/src/main/java/com/facebook/openwifirrm/modules/ConfigManager.java b/src/main/java/com/facebook/openwifirrm/modules/ConfigManager.java index 91eacf9..f2389da 100644 --- a/src/main/java/com/facebook/openwifirrm/modules/ConfigManager.java +++ b/src/main/java/com/facebook/openwifirrm/modules/ConfigManager.java @@ -10,9 +10,12 @@ package com.facebook.openwifirrm.modules; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import org.slf4j.Logger; @@ -63,8 +66,11 @@ public class ConfigManager implements Runnable { /** Is the main thread sleeping? */ private final AtomicBoolean sleepingFlag = new AtomicBoolean(false); - /** Was a manual config update requested? */ - private final AtomicBoolean eventFlag = new AtomicBoolean(false); + /** + * Thread-safe set of zones for which manual config updates have been + * requested. + */ + private Set zonesToUpdate = ConcurrentHashMap.newKeySet(); /** Config listener interface. */ public interface ConfigListener { @@ -180,7 +186,10 @@ public class ConfigManager implements Runnable { List devicesNeedingUpdate = new ArrayList<>(); final long CONFIG_DEBOUNCE_INTERVAL_NS = params.configDebounceIntervalSec * 1_000_000_000L; - final boolean isEvent = eventFlag.getAndSet(false); + Set zonesToUpdateCopy = new HashSet<>(zonesToUpdate); + // use removeAll() instead of clear() in case items are added between + // the previous line and the following line + zonesToUpdate.removeAll(zonesToUpdateCopy); for (DeviceWithStatus device : devices) { // Update config structure DeviceData data = deviceDataMap.computeIfAbsent( @@ -201,11 +210,13 @@ public class ConfigManager implements Runnable { for (ConfigListener listener : configListeners.values()) { listener.receiveDeviceConfig(device.serialNumber, data.config); } - - // Check event flag + // Check if there are requested updates for this zone + String deviceZone = + deviceDataManager.getDeviceZone(device.serialNumber); + boolean isEvent = zonesToUpdateCopy.contains(deviceZone); if (params.configOnEventOnly && !isEvent) { logger.debug( - "Skipping config for {} (event flag not set)", + "Skipping config for {} (zone not marked for updates)", device.serialNumber ); continue; @@ -251,15 +262,16 @@ public class ConfigManager implements Runnable { } } + final boolean shouldUpdate = !zonesToUpdateCopy.isEmpty(); // Send config changes to devices if (!params.configEnabled) { logger.trace("Config changes are disabled."); } else if (devicesNeedingUpdate.isEmpty()) { logger.debug("No device configs to send."); - } else if (params.configOnEventOnly && !isEvent) { + } else if (params.configOnEventOnly && !shouldUpdate) { // shouldn't happen logger.error( - "ERROR!! {} device(s) queued for config update, but event flag not set", + "ERROR!! {} device(s) queued for config update, but no zones queued for update.", devicesNeedingUpdate.size() ); } else { @@ -364,9 +376,38 @@ public class ConfigManager implements Runnable { return (configListeners.remove(id) != null); } - /** Interrupt the main thread, possibly triggering an update immediately. */ - public void wakeUp() { - eventFlag.set(true); + /** + * Mark the zone to be updated, then interrupt the main thread to possibly + * trigger an update immediately. + * + * @param zone non-null zone (i.e., venue) + */ + public void queueZoneAndWakeUp(String zone) { + if (zone == null) { + logger.debug("Zone to queue must be a non-null String."); + return; + } + zonesToUpdate.add(zone); + wakeUp(); + } + + /** + * Track all zones to be updated, then interrupt the main thread to possibly + * trigger an update immediately. + */ + public void queueAllZonesAndWakeUp() { + /* + * Note, addAll is not atomic, but that is ok. This just means that it + * is possible that some zones may get updated now by the main thread + * while others get updated either when the main thread is woken up or + * the next time the main thread does its periodic update. + */ + zonesToUpdate.addAll(deviceDataManager.getZones()); + wakeUp(); + } + + /** Interrupt the main thread to possibly trigger an update immediately. */ + private void wakeUp() { if (mainThread != null && mainThread.isAlive() && sleepingFlag.get()) { wakeupFlag.set(true); mainThread.interrupt(); diff --git a/src/main/java/com/facebook/openwifirrm/modules/RRMScheduler.java b/src/main/java/com/facebook/openwifirrm/modules/RRMScheduler.java index 0a708e1..70e0725 100644 --- a/src/main/java/com/facebook/openwifirrm/modules/RRMScheduler.java +++ b/src/main/java/com/facebook/openwifirrm/modules/RRMScheduler.java @@ -8,15 +8,15 @@ package com.facebook.openwifirrm.modules; +import java.text.ParseException; import java.util.Arrays; import java.util.HashSet; import java.util.Properties; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.text.ParseException; -import org.quartz.CronScheduleBuilder; import org.quartz.CronExpression; +import org.quartz.CronScheduleBuilder; import org.quartz.Job; import org.quartz.JobBuilder; import org.quartz.JobDetail; @@ -332,7 +332,8 @@ public class RRMScheduler { modeler, zone, params.dryRun, - true /* allowDefaultMode */ + true, /* allowDefaultMode */ + false /* updateImmediately */ ); logger.info( "'{}' result for zone '{}': {}", @@ -341,5 +342,6 @@ public class RRMScheduler { gson.toJson(result) ); } + configManager.queueZoneAndWakeUp(zone); } } diff --git a/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java b/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java index 9846c25..aaff2d8 100644 --- a/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java +++ b/src/main/java/com/facebook/openwifirrm/optimizers/channel/ChannelOptimizer.java @@ -638,14 +638,13 @@ public abstract class ChannelOptimizer { public abstract Map> computeChannelMap(); /** - * Program the given channel map into the AP config and notify the config - * manager. + * Program the given channel map into the AP config. * * @param deviceDataManager the DeviceDataManager instance * @param configManager the ConfigManager instance * @param channelMap the map of devices (by serial number) to radio to channel */ - public void applyConfig( + public void updateDeviceApConfig( DeviceDataManager deviceDataManager, ConfigManager configManager, Map> channelMap @@ -663,8 +662,5 @@ public abstract class ChannelOptimizer { deviceConfig.autoChannels = entry.getValue(); } }); - - // Trigger config update now - configManager.wakeUp(); } } diff --git a/src/main/java/com/facebook/openwifirrm/optimizers/tpc/TPC.java b/src/main/java/com/facebook/openwifirrm/optimizers/tpc/TPC.java index df4eb50..ff93af9 100644 --- a/src/main/java/com/facebook/openwifirrm/optimizers/tpc/TPC.java +++ b/src/main/java/com/facebook/openwifirrm/optimizers/tpc/TPC.java @@ -151,14 +151,13 @@ public abstract class TPC { public abstract Map> computeTxPowerMap(); /** - * Program the given tx power map into the AP config and notify the config - * manager. + * Program the given tx power map into the AP config. * * @param deviceDataManager the DeviceDataManager instance * @param configManager the ConfigManager instance * @param txPowerMap the map of devices (by serial number) to radio to tx power */ - public void applyConfig( + public void updateDeviceApConfig( DeviceDataManager deviceDataManager, ConfigManager configManager, Map> txPowerMap @@ -176,9 +175,6 @@ public abstract class TPC { deviceConfig.autoTxPowers = entry.getValue(); } }); - - // Trigger config update now - configManager.wakeUp(); } /**