Skip to content

Fix locking issue and optimize a bit#3

Open
suutari-ai wants to merge 2 commits intoandersinno:masterfrom
suutari-ai:fix-locking
Open

Fix locking issue and optimize a bit#3
suutari-ai wants to merge 2 commits intoandersinno:masterfrom
suutari-ai:fix-locking

Conversation

@suutari-ai
Copy link
Member

Fix an issue with locking by creating just a single PePuRunner on the
process start up in run_pepubot function.

Previously a new PePuRunner was created for each incoming Slack message
and the locking of the shared resources (e.g. runner state and the
lottery box) was done with an instance attribute of the
PePuRunner. Therefore there was actually a different lock for each
PePuRunner and the locking wasn't able to protect two distinct message
processing flows to mess up each other.

Also add an optimization that skips the state loading when its already
loaded.

Fixes #2

Fix an issue with locking by creating just a single PePuRunner on the
process start up in run_pepubot function.

Previously a new PePuRunner was created for each incoming Slack message
and the locking of the shared resources (e.g. runner state and the
lottery box) was done with an instance attribute of the PePuRunner.
Therefore there was actually a different lock for each PePuRunner and
the locking wasn't able to protect two distinct message processing flows
to mess up each other.
There is no need to load the state of the PePuRunner more than once.
Now that there is only a single PePuRunner instantiated, this actually
makes a difference too.
@codecov
Copy link

codecov bot commented Oct 13, 2019

Codecov Report

Merging #3 into master will decrease coverage by 0.45%.
The diff coverage is 30.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #3      +/-   ##
==========================================
- Coverage   34.16%   33.71%   -0.46%     
==========================================
  Files          11       11              
  Lines         518      522       +4     
  Branches       74       75       +1     
==========================================
- Hits          177      176       -1     
- Misses        340      345       +5     
  Partials        1        1
Impacted Files Coverage Δ
pepubot/__main__.py 38.7% <28.57%> (-6.46%) ⬇️
pepubot/runner.py 23.59% <33.33%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b3f9a6...2cf4fb8. Read the comment docs.

@suutari-ai suutari-ai requested a review from frwickst October 13, 2019 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The locking is done incorrectly is PePuRunner

1 participant