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

render-test-flakiness:clear worker storage #11111

Merged
merged 8 commits into from
Oct 12, 2021

Conversation

arindam1993
Copy link
Contributor

@arindam1993 arindam1993 commented Oct 11, 2021

Clears the mapId keyed caches on the worker so memory is released when prewarm() is used.

This should help keep runaway memory growth when running render tests on CI in check.

Closes #10888

Copy link
Member

@mourner mourner left a comment

Choose a reason for hiding this comment

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

Makes a lot of sense! How's the flakiness looking with multiple runs now?

@arindam1993
Copy link
Contributor Author

arindam1993 commented Oct 12, 2021

@mourner Still not good :(
I think theres also an issue with IO/file system thrashing because the node server is getting bombarded with actual.png's to write while also having to serve assets
Screen Shot 2021-10-12 at 9 24 10 AM
The most common cause of error seems like its coming from tile loading, I'm going to test by skipping the writing of actual.pngs on CI

@arindam1993
Copy link
Contributor Author

arindam1993 commented Oct 12, 2021

That (not writing actual.png and thrashing IO) seems to have worked, I've had 6 continuous successful runs. I also updated it to move away from the pinned version of chrome, so this closes #10888.

Had one flake happen, but its not like the usual flaking we see with exceptions being thown, instead its this one variable anchor test with a difference in the symbol placement that I think is known to be a bit flaky
Screen Shot 2021-10-12 at 11 28 29 AM
.

@karimnaaji
Copy link
Contributor

karimnaaji commented Oct 12, 2021

That (not writing actual.png and thrashing IO) seems to have worked, I've had 6 continuous successful runs. I also updated it to move away from the pinned version of chrome, so this closes #10888.

Had one flake happen, but its not like the usual flaking we see with exceptions being thown, instead its this one variable anchor test with a difference in the symbol placement that I think is known to be a bit flaky

Nice! Should we put this one in the ignore list and have a reference issue for it?

@arindam1993
Copy link
Contributor Author

I think its fine for now, we can follow up later if these 1-off flakes end up being too annoying.

@arindam1993 arindam1993 merged commit 1c416c6 into main Oct 12, 2021
@arindam1993 arindam1993 deleted the render-test-flakiness/clear-workers branch October 12, 2021 19:05
katydecorah pushed a commit that referenced this pull request Oct 20, 2021
* main:
  Add touch pan blocker to gesture handling for touch devices (#11116)
  Address accessibility issues (#11064)
  add support for non-mercator projections (#11124)
  Image fallback expressions within paint properties (#11049)
  Replaces EPSG:4326 with OGC:CRS84 in GL JS `LngLat` doc (#11072)
  Add globe view support to heatmap shaders (#11120)
  Exclude flaky test (#11118)
  consistify YOUR_MAPBOX_ACCESS_TOKEN as placeholder string (#11113)
  Allow adding multiple layers to `map.on()` event handler (h/t @omerbn) (#11114)
  render-test-flakiness:clear worker storage (#11111)
  upgrade to supercluster v7.1.4, earcut v2.2.3, vt-pbf v3.1.3, geojson-rewind v0.5.1 (#11110)
  Added v1.13.2 changelog entry (#11108)
  One weird JSON.parse() trick (#11098)
  Fixed doc usage of map.getCenter (#11093)
  s̶y̶m̶b̶o̶l̶-̶c̶l̶i̶p̶ dynamic-filtering with `pitch` and `distance-from-camera` expressions (#10795)
  Update link to transpiling guide (#11096)
  Cherry pick 2.5.1 changelog (#11099)
  Fix an iOS15 issue where Safari tab bar interrupts panning (#11084)
  Fix conditional check for isFullscreen to accommodate Safari (#11086)
  Render tests for #11041 (#11070)
karimnaaji added a commit that referenced this pull request Oct 21, 2021
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.

Unpin Chrome version on CI from version 91
3 participants