Skip to content

Commit

Permalink
fix stirring not cancelling correctly and race conditions
Browse files Browse the repository at this point in the history
  • Loading branch information
CamDavidsonPilon committed Dec 4, 2024
1 parent 8231810 commit b410e91
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 26 deletions.
13 changes: 6 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@

#### Enhancements
- improvements to Dodging background job code, including the ability to initialize the class based on dodging or not.
- better error handling for failed OD blanks.
- better error handling for failed OD blank action.
- better button state management in the UI.
- job YAMLs' published_settings can have a new field, `editable` (bool), which controls whether it shows up on the Settings dialog or not. (False means it won't show up since it's not editable!). Default is true. This _should_ align with the `published_setting` in Python's job classes.
- a job YAMLs' published_settings can have a new field, `editable` (bool), which controls whether it shows up on the Settings dialog or not. (False means it won't show up since it's not editable!). Default is true. This _should_ align with the `published_setting` in Python's job classes.
- you can add IPv4 addresses to the (new) `[cluster.addresses]` section to specify IPs for pioreactors. Example:
```
[cluster.addresses]
pio01_address=10.42.0.2
pio02_address=10.42.0.3
[cluster.addresses]
pio01=10.42.0.2
pio02=10.42.0.3
```
Note that the leader's address is automatically added in our software.
Expand All @@ -28,7 +27,7 @@
- Fixed "circulate X" actions in the Manage All dialog in the UI.

#### Breaking changes
- moved all the temporary caches, which previously where their own sqlite3 db in /tmp/ to /tmp/local_intermittent_pioreactor_metadata.sqlite. This shouldn't break anything unless you update _during_ an experiment - don't do that!
- moved all the temporary caches, which previously where their own sqlite3 dbs in `/tmp/` to `/tmp/local_intermittent_pioreactor_metadata.sqlite`. This shouldn't break anything unless you update _during_ an experiment - don't do that!

### 24.10.29

Expand Down
15 changes: 6 additions & 9 deletions pioreactor/background_jobs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -1053,8 +1053,12 @@ def __post__init__(self):
self.set_enable_dodging_od(
config.getboolean(f"{self.job_name}.config", "enable_dodging_od", fallback="False")
)
self.start_passive_listeners()
super().__post__init__()
# now that `enable_dodging_od` is set, we can check for OD
self.subscribe_and_callback(
self._od_reading_changed_status,
f"pioreactor/{self.unit}/{self.experiment}/od_reading/interval",
)
super().__post__init__() # set ready

def set_currently_dodging_od(self, value: bool):
self.currently_dodging_od = value
Expand Down Expand Up @@ -1094,13 +1098,6 @@ def initialize_dodging_operation(self) -> None:
def initialize_continuous_operation(self) -> None:
pass

def _start_general_passive_listeners(self) -> None:
super()._start_general_passive_listeners()
self.subscribe_and_callback(
self._od_reading_changed_status,
f"pioreactor/{self.unit}/{self.experiment}/od_reading/interval",
)

def _od_reading_changed_status(self, msg):
if self.enable_dodging_od:
# only act if our internal state is discordant with the external state
Expand Down
7 changes: 5 additions & 2 deletions pioreactor/background_jobs/stirring.py
Original file line number Diff line number Diff line change
Expand Up @@ -281,18 +281,21 @@ def action_to_do_after_od_reading(self):
self.poll_and_update_dc()

def initialize_dodging_operation(self):
if config.getfloat("od_reading.config", "samples_per_second") > 0.121:
if config.getfloat("od_reading.config", "samples_per_second") > 0.12:
self.logger.warning(
"Recommended to decrease `samples_per_second` to ensure there is time to start/stop stirring. Try 0.12 or less."
)

with suppress(AttributeError):
self.rpm_check_repeated_thread.cancel()

self.rpm_check_repeated_thread = RepeatedTimer(
1_000,
lambda *args: None,
job_name=self.job_name,
logger=self.logger,
)
self.stop_stirring() # we'll start it in action_to_do_after_od_reading
self.stop_stirring() # we'll start it again in action_to_do_after_od_reading

def initialize_continuous_operation(self):
# set up thread to periodically check the rpm
Expand Down
2 changes: 1 addition & 1 deletion pioreactor/cli/pio.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,7 +531,7 @@ def update_app(
logger.error("Update failed. See logs.")
# end early
raise click.Abort()
else:
elif p.stdout:
logger.debug(p.stdout)

logger.notice(f"Updated Pioreactor app to version {version_installed}.") # type: ignore
Expand Down
2 changes: 1 addition & 1 deletion pioreactor/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def get_config() -> ConfigParserMod:

leader_hostname = config.get("cluster.topology", "leader_hostname")
leader_address = config.get("cluster.topology", "leader_address")
config.set("cluster.addresses", f"{leader_hostname}_address", leader_address)
config.set("cluster.addresses", leader_hostname, leader_address)

return config

Expand Down
2 changes: 1 addition & 1 deletion pioreactor/utils/networking.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def worker_hostnames(queue: Queue) -> None:
def resolve_to_address(hostname: str) -> str:
# TODO: make this more fleshed out: resolve to IP, etc.
# add_local assumes a working mDNS.
address_in_config = config.get("cluster.addresses", f"{hostname}_address", fallback=None)
address_in_config = config.get("cluster.addresses", hostname, fallback=None)
if address_in_config is not None:
return address_in_config
else:
Expand Down
3 changes: 2 additions & 1 deletion pioreactor/utils/timing.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ def unpause(self) -> None:
self.is_paused = False

def cancel(self, timeout: t.Optional[float] = None) -> None:
self.event.set()
self.pause() # this will exit from the _target early.
self.event.set() # stop waiting in _target.

with suppress(RuntimeError):
# possible to happen if self.thread hasn't started yet,
Expand Down
Binary file modified update_scripts/upcoming/datasets.zip
Binary file not shown.
2 changes: 1 addition & 1 deletion update_scripts/upcoming/install_pioreactor_plugin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fi
if [ "$am_i_leader" = true ]; then
# merge new config.ini
if test -f "$install_folder/additional_config.ini"; then
crudini --merge /home/pioreactor/.pioreactor/config.ini < "$install_folder/additional_config.ini"
crudini --ini-options=nospace --merge /home/pioreactor/.pioreactor/config.ini < "$install_folder/additional_config.ini"
fi

# add any new sql, restart mqtt_to_db job, too
Expand Down
6 changes: 3 additions & 3 deletions update_scripts/upcoming/update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ HOSTNAME=$(hostname)

# Get the leader hostname
# Don't use `leader_address`, as users do change that.
LEADER_HOSTNAME=$(crudini --get "$PIO_DIR"/config.ini cluster.topology leader_hostname)
LEADER_HOSTNAME=$(crudini --get /home/pioreactor/.pioreactor/config.ini cluster.topology leader_hostname)

if [ "$HOSTNAME" = "$LEADER_HOSTNAME" ]; then
crudini --set /home/pioreactor/.pioreactor/config.ini cluster_addresses \
Expand All @@ -37,5 +37,5 @@ if [ "$HOSTNAME" = "$LEADER_HOSTNAME" ]; then

fi

sudo -u pioreactor cp "$SCRIPT_DIR/install_pioreactor_plugin.sh" /usr/local/bin/install_pioreactor_plugin.sh
sudo -u pioreactor cp "$SCRIPT_DIR/uninstall_pioreactor_plugin.sh" /usr/local/bin/uninstall_pioreactor_plugin.sh
cp "$SCRIPT_DIR/install_pioreactor_plugin.sh" /usr/local/bin/install_pioreactor_plugin.sh
cp "$SCRIPT_DIR/uninstall_pioreactor_plugin.sh" /usr/local/bin/uninstall_pioreactor_plugin.sh

0 comments on commit b410e91

Please sign in to comment.