Retrying renamedTo operation up to 5 times because it randomly fails …#11216
Retrying renamedTo operation up to 5 times because it randomly fails …#11216a-zitzewitz wants to merge 5 commits intojenkinsci:masterfrom
Conversation
…on slow file systems.
|
Yay, your first pull request towards Jenkins core was created successfully! Thank you so much! |
|
The failing tests are not related to my changes. They only occurred after merging the main branch. |
timja
left a comment
There was a problem hiding this comment.
Seems fine, unlikely to happen in most cases so shouldn't get invoked except in edge cases.
|
Could you file an issue in https://github.com/jenkins-infra/helpdesk/ about this? Even if there's no straightforward infra-side fix, it may help to record this problem. This fix would take a while to arrive at your plugin unless you limit who gets to install it by increasing the core dependency to the next weekly release. |
|
Did you confirm this resolves the issue for you? You can have a plugin PR depend on this core PR by specifying what was uploaded in https://github.com/jenkinsci/jenkins/runs/53292020888 as e.g. |
|
@daniel-beck Infrastructure issue created: jenkins-infra/helpdesk#4854 |
|
@daniel-beck The problem is that the tests are only failing on your build infrastructure. Maybe an issue with file system configuration? I don't know. But these issues are blocking me from creating a new release of the plugin. |
but in the plugin change its |
|
@timja Tried your proposal to test it with the Jenkins version created by the PR. It fails, because this version does not include the chmod fix of @strangelookingnerd . So I don't even get to the Windows build, because the Linux build fails due t the chmod issue. |
|
Easiest way is to create a separate test PR in draft that includes the other fix. See this comment for how to have a fast build for just getting the incremental version: |
|
Can we get this merged? Otherwise my tests will not succeed on the Jenkins CI server and I will never be able to update my plugin. For the tests to work I need @strangelookingnerd changes for the chmod fix and this PR integrated. I cannot even test this in isolation, because the version generated by this PR does not include @strangelookingnerd 's changes. So it fails die to chmod and not rename. |
|
@strangelookingnerd 's change is merged so you should be able to check that this works. |
Sorry for the stupid question, but where do I find the Jenkins version I need to use for the test? |
|
Version is |
|
@timja I can confirm that the rename problem is now fixed. |
|
Noting that jenkinsci/jenkins-infra-test-plugin#195 / jenkinsci/sonargraph-integration-plugin#36 did not fail to download/extract in standalone tests, so it's not quite as easy as "Jenkins infra bad". There may be something wrong with the tests, or the plugins used in the tests. This PR and the other may just hide a real problem. |

…on slow file systems.
See JENKINS-76192.
While trying to make all test pass for the sonargraph-integration-plugin I found that the tests will often (not always) fail when installing a tool on a windows node. It is not always the same test that fails, but always the same stack trace:
That implies that it is not a bug in the plugin, but in the tool installation operation. It seems that freshly created files are not visible immediately for other operations like rename. Therefore the proposed changed will retry the rename operation 5 times before giving up and throwing an
IOException. That should give the file system enough time to catch up.Testing done
All tests pass, change is covered by existing tests
Proposed changelog entries
Proposed changelog category
/remove-label major-bug
/label bug
Proposed upgrade guidelines
N/A
Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.evalto ease future introduction of Content Security Policy (CSP) directives (see documentation).Desired reviewers
@strangelookingnerd
@jenkinsci/core-pr-reviewer
Before the changes are marked as
ready-for-merge:Maintainer checklist
upgrade-guide-neededlabel is set and there is a Proposed upgrade guidelines section in the pull request title (see example).lts-candidateto be considered (see query).