Skip to content

Stop passing URL opener across process boundaries#1820

Closed
nanonyme wants to merge 1 commit into
apache:masterfrom
nanonyme:nanonyme/downloadable-source
Closed

Stop passing URL opener across process boundaries#1820
nanonyme wants to merge 1 commit into
apache:masterfrom
nanonyme:nanonyme/downloadable-source

Conversation

@nanonyme

Copy link
Copy Markdown
Contributor

Should fix #1766

The object is not picklable. This cannot be supported. Instead create URL opener when needed in child process. Drops URL opener caching which may result in minor performance impact.

@nanonyme nanonyme force-pushed the nanonyme/downloadable-source branch 7 times, most recently from 7fdcd4f to 3c616a4 Compare January 29, 2023 16:13
@nanonyme nanonyme marked this pull request as ready for review January 29, 2023 16:17
@nanonyme

Copy link
Copy Markdown
Contributor Author

Looks like this breaks netrfc functionality. Need to debug.

@nanonyme

Copy link
Copy Markdown
Contributor Author

I suspect this is CI-only problem. Environment is working in unexpected way.

@nanonyme nanonyme force-pushed the nanonyme/downloadable-source branch 6 times, most recently from bbaf278 to 745e70a Compare January 29, 2023 17:13
@nanonyme

Copy link
Copy Markdown
Contributor Author

Okay, done. I think there is some problem with buildgrid that is causing test failures.

@BenjaminSchubert BenjaminSchubert left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the change, but I am a bit cautious about the invalid netrc error, I am not sure I get why this can't be logged anymore?

Comment thread src/buildstream/downloadablefilesource.py Outdated
Comment thread src/buildstream/downloadablefilesource.py Outdated
@nanonyme nanonyme force-pushed the nanonyme/downloadable-source branch 2 times, most recently from f20ff8d to 7b9e39d Compare February 11, 2023 15:12
The object is not picklable. This cannot be supported. Instead create
URL opener when needed in child process. Drops URL opener caching
which may result in minor performance impact.
@nanonyme nanonyme force-pushed the nanonyme/downloadable-source branch from 7b9e39d to 5750589 Compare February 11, 2023 15:16
@nanonyme

Copy link
Copy Markdown
Contributor Author

Failing test appears to have been failed at [00:00:00] FAILURE [4a47c98a] build-shell/buildtree.bst: Failed to link '/home/testuser/buildstream/.tox/py310/tmp/test_shell_pull_cached_buildtr2/cache/artifacts/refs/test/build-shell-buildtree/4a47c98a10df39e65e99d471f96edc5b58d4ea5b9b1f221d0be832a8124b8099 -> /home/testuser/buildstream/.tox/py310/tmp/test_shell_pull_cached_buildtr2/cache/artifacts/refs/test/build-shell-buildtree/38b730aec720dbddba843c9e3ce1ad8073e39d16cc4d25f752bc89d867fbaec1': [Errno 17] File exists: '/home/testuser/buildstream/.tox/py310/tmp/test_shell_pull_cached_buildtr2/cache/artifacts/refs/test/build-shell-buildtree/4a47c98a10df39e65e99d471f96edc5b58d4ea5b9b1f221d0be832a8124b8099' -> '/home/testuser/buildstream/.tox/py310/tmp/test_shell_pull_cached_buildtr2/cache/artifacts/refs/test/build-shell-buildtree/38b730aec720dbddba843c9e3ce1ad8073e39d16cc4d25f752bc89d867fbaec1'

@abderrahim

Copy link
Copy Markdown
Contributor

Failing test appears to have been failed at [...]

Which seems to be the same as #1814. I guess that's just a flaky test.

@nanonyme

Copy link
Copy Markdown
Contributor Author

It looks strange. Apparently the error originates here https://github.com/apache/buildstream/blob/master/src/buildstream/utils.py#L432 and it looks like bst should upon file existing try to delete it and then try to link again.

@nanonyme

nanonyme commented Feb 12, 2023

Copy link
Copy Markdown
Contributor Author

It definitely looks like a race condition with multiple parties trying create same link. Supposedly a chain

  1. A creates link
  2. B tries to create link and gets error it exists
  3. C tries to create link and gets error it exists
  4. B deletes link
  5. C tries to delete link and silently fails
  6. B creates link
  7. C tries to create link and throws exception

can result in this failure. safe_link is not multi-thread or multi-process safe.

@nanonyme

Copy link
Copy Markdown
Contributor Author

The should probably be a follow-up issue on that. I suspect there might be something specific to test runner that results in this since I haven't ever seen this in real world.

@gtristan

Copy link
Copy Markdown
Contributor

Rebased this and fixed conflicts with downloadablefilesource.py, this PR is now replaced by #1826

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.

Traceback bug in artifact fetch

4 participants