Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve issue where a spawn process' slab fails #1914

Merged
merged 5 commits into from
Oct 14, 2020
Merged

Resolve issue where a spawn process' slab fails #1914

merged 5 commits into from
Oct 14, 2020

Conversation

jnwatson
Copy link
Contributor

Previously, if a main process' slab mapsize increases, any existing spawn processes (which have the same slabs open readonly) would get a lmdb.MapResizedError when calling slab.initdb.

Now, initdb correctly handles that case.

Previously, if a main process' slab mapsize increases, any existing spawn
processes (which have the same slabs open readonly) would get a
lmdb.MapResizedError when calling slab.initdb.

Now, initdb correctly handles that case.
@codecov
Copy link

codecov bot commented Oct 13, 2020

Codecov Report

Merging #1914 into master will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1914      +/-   ##
==========================================
- Coverage   96.70%   96.61%   -0.09%     
==========================================
  Files         139      139              
  Lines       26467    26470       +3     
==========================================
- Hits        25595    25575      -20     
- Misses        872      895      +23     
Flag Coverage Δ
#linux 96.61% <100.00%> (-0.09%) ⬇️
#linux_replay 70.95% <33.33%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
synapse/lib/lmdbslab.py 99.39% <100.00%> (-0.21%) ⬇️
synapse/lib/spawn.py 89.10% <0.00%> (-6.42%) ⬇️
synapse/common.py 97.16% <0.00%> (-0.32%) ⬇️

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 78dd656...884c824. Read the comment docs.

proc.join(10)
self.nn(proc.exitcode)
slab.initdb('foo')
self.true(True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this just testing that the code doesn't go boom?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patch coverage of 100% seems to indicate that it covers triggering the actual resize, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This tests that the previous line didn't hit an exception (which it did before the change).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(It is redundant, but I know you like the explicitness)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@vEpiphyte vEpiphyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved once CI gremlins are done doing their magick..

@vEpiphyte
Copy link
Contributor

Added patches to increase the timeout for the Spawn stormpkg test and a patch from Neomorph for a teardown race on the Slab when we delete the environment.

@jnwatson jnwatson merged commit 9734718 into master Oct 14, 2020
@jnwatson jnwatson deleted the spawn-lmdb branch October 14, 2020 16:11
@vEpiphyte vEpiphyte modified the milestones: 2.10.0, 2.9.0 Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants