Uploaded image for project: 'Bitbucket Data Center'
  1. Bitbucket Data Center
  2. BSERV-7360

Front end for pull request merges should wait as long as server does for process to complete

    • We collect Bitbucket feedback from various sources, and we evaluate what we've collected when planning our product roadmap. To understand how this piece of feedback will be reviewed, see our Implementation of New Features Policy.

      Problem Definition

      If you have a large repository containing binary files merges often take longer than a minute. The error "Server Unreachable" is given even though the merge is still occurring behind the scenes.

      Description on https://confluence.atlassian.com/display/STASHKB/Server+Unreachable+-+Timeout+when+merging+or+viewing+Diff+of+pull+request

      Cause

      The server allows the merge up to 5m (configurable) to complete and the UI (not configurable) would have to be willing to wait the same amount of time the server is willing to wait but it is currently hard-coded (1 minute).

      Suggested Solution

      It would be good to have a more informative message for these cases, or at least the ability to turn off the error message so that developers don't think that something has gone wrong. A loading bar for these large merges would be ideal.

          Form Name

            [BSERV-7360] Front end for pull request merges should wait as long as server does for process to complete

            Gordon,

            Git 2.2.x and 2.3.x (all versions) have a known performance issue on NFS. Here's a thread on Nabble about the issue, raised by our own ssaasen. On our NFS-based instances, upgrading to Git 2.5 (or downgrading to Git 2.1.4, which is the last release before the performance regression was introduced) produced extremely significant improvements in merge times. The fix for that performance issue is present in 2.4.7-2.4.10 and all versions from 2.5.0 forward. So if you're using NFS and Git 2.3.x, there's every chance it is an NFS issue. I'm not saying upgrading will make your merges take a few seconds, but there's a good chance it will make them faster.

            With respect to merges, I can see where you'd get the idea that's how Stash's merges and pull request creation work, but it's actually not. Allow me to clarify.

            When you open a pull request via the UI, you're redirected to the new pull request's overview screen. When you arrive on that screen, the UI makes a REST call to check whether the pull request is mergeable (or has conflicts). That triggers the server to do a "dark" merge of the pull request. That same "dark" merge is used to display the pull request's diff. Unlike other tools, Stash and Bitbucket Server show you the diff of an actual merge, showing you what your changes will look like when applied to the target branch rather than showing you the changes that have been made on the source branch. That's how the system is able to show merge conflicts; you're looking at the actual merge result.

            That "dark" merge is 100% separate from the actual merge that happens when you click the "Merge" button. Even the code that creates the two merges is not the same.

            The "Merge" button is enabled by default when you view a pull request. This is based on two assumptions which are generally true:

            • Most pull requests merge cleanly most of the time
            • The delay for the "dark" merge to happen and report conflicts is usually fairly small

            If the "dark" merge reports conflicts, the "Merge" button is disabled. The system caches that "dark" merge, so subsequent views of the pull request, assuming it has not been updated, should disable the "Merge" button very quickly, even if the "dark" merge originally took multiple minutes to perform. On that very first view, though, if your merges are quite slow the "Merge" button might remain enabled for a noticeable period of time before flipping to "disabled".

            Best regards,
            Bryan Turner
            Atlassian Bitbucket

            (Please note, I'm just trying to provide some background and raise awareness of a potential performance issue. None of my comments here should be taken to indicate I don't think this is a real issue or to indicate that this issue will not be resolved.)

            Bryan Turner (Inactive) added a comment - Gordon, Git 2.2.x and 2.3.x ( all versions ) have a known performance issue on NFS. Here's a thread on Nabble about the issue, raised by our own ssaasen . On our NFS-based instances, upgrading to Git 2.5 (or downgrading to Git 2.1.4, which is the last release before the performance regression was introduced) produced extremely significant improvements in merge times. The fix for that performance issue is present in 2.4.7-2.4.10 and all versions from 2.5.0 forward. So if you're using NFS and Git 2.3.x, there's every chance it is an NFS issue. I'm not saying upgrading will make your merges take a few seconds, but there's a good chance it will make them faster. With respect to merges, I can see where you'd get the idea that's how Stash's merges and pull request creation work, but it's actually not. Allow me to clarify. When you open a pull request via the UI, you're redirected to the new pull request's overview screen. When you arrive on that screen, the UI makes a REST call to check whether the pull request is mergeable (or has conflicts). That triggers the server to do a "dark" merge of the pull request. That same "dark" merge is used to display the pull request's diff. Unlike other tools, Stash and Bitbucket Server show you the diff of an actual merge, showing you what your changes will look like when applied to the target branch rather than showing you the changes that have been made on the source branch. That's how the system is able to show merge conflicts; you're looking at the actual merge result. That "dark" merge is 100% separate from the actual merge that happens when you click the "Merge" button. Even the code that creates the two merges is not the same. The "Merge" button is enabled by default when you view a pull request. This is based on two assumptions which are generally true : Most pull requests merge cleanly most of the time The delay for the "dark" merge to happen and report conflicts is usually fairly small If the "dark" merge reports conflicts, the "Merge" button is disabled. The system caches that "dark" merge, so subsequent views of the pull request, assuming it has not been updated, should disable the "Merge" button very quickly, even if the "dark" merge originally took multiple minutes to perform. On that very first view, though, if your merges are quite slow the "Merge" button might remain enabled for a noticeable period of time before flipping to "disabled". Best regards, Bryan Turner Atlassian Bitbucket (Please note, I'm just trying to provide some background and raise awareness of a potential performance issue. None of my comments here should be taken to indicate I don't think this is a real issue or to indicate that this issue will not be resolved.)

            Bryan

            We've actually found that a/ people do sit at the browser screen when they are doing critical PRs and b/ the issue is that the UI pops up a misleading error and c/ the UI doesn't currently repent them getting ahead of themselves (pressing Merge thinking the PR is ready to go) and generating more UI errors...

            We have a large git repo containing many binary files (test results) and a merge takes ~5 minutes. I've tried it on many machines, SSDs etc. its not a NFS issue. We are on git 2.3.x between stash and various clients. Our production VCS box is actually a beast.

            Stash seems to work like this (which indicates that the UI merge button isn't quite what it looks like) although you would know best:

            • create PR - stash does a clone and a merge in the background - with no UI notifications except errors - the merge button is available even even if the background activity hasn't completed (i'm unclear where reviewer email is sent in this process)
            • hit merge button - stash pushes the cloned merge to the true repo - with no UI notifications except at the end

            Stash does not prevent the merge button being hit while the PR is still being merged, not let the UI know that something is still in progress.

            I have many other repos where transactions are fast and this is not an issue, but for the bug repos it is.

            On idea is a change to the UI to actually display "PR creation in progress" and disabling the merge button, and then having a "merge in progress" (also with disabled button) would make this issue go away completely, along with making sure that the UI timeouts can never be less than server-backend timeouts. These UI states might be very fast for small repos, but we've had to change the overall stash config to allow for the big repos so it seems wise...

            Gordon Waddell added a comment - Bryan We've actually found that a/ people do sit at the browser screen when they are doing critical PRs and b/ the issue is that the UI pops up a misleading error and c/ the UI doesn't currently repent them getting ahead of themselves (pressing Merge thinking the PR is ready to go) and generating more UI errors... We have a large git repo containing many binary files (test results) and a merge takes ~5 minutes. I've tried it on many machines, SSDs etc. its not a NFS issue. We are on git 2.3.x between stash and various clients. Our production VCS box is actually a beast. Stash seems to work like this (which indicates that the UI merge button isn't quite what it looks like) although you would know best: create PR - stash does a clone and a merge in the background - with no UI notifications except errors - the merge button is available even even if the background activity hasn't completed (i'm unclear where reviewer email is sent in this process) hit merge button - stash pushes the cloned merge to the true repo - with no UI notifications except at the end Stash does not prevent the merge button being hit while the PR is still being merged, not let the UI know that something is still in progress. I have many other repos where transactions are fast and this is not an issue, but for the bug repos it is. On idea is a change to the UI to actually display "PR creation in progress" and disabling the merge button, and then having a "merge in progress" (also with disabled button) would make this issue go away completely, along with making sure that the UI timeouts can never be less than server-backend timeouts. These UI states might be very fast for small repos, but we've had to change the overall stash config to allow for the big repos so it seems wise...

            Is this problem solved?

            Xiangqian Lee added a comment - Is this problem solved?

            Bryan Turner (Inactive) added a comment - - edited

            Gordon,

            Do you actually have pull requests that take 30 minutes to complete a merge? Because I think the reality is that regardless of whether the UI waits for that or not, there's zero chance the human setting at their browser's going to wait that long. If you need the timeouts to be 6 or 7 times the default value I'd suggest opening a support ticket and working with us to determine why your merges are so slow. (NFS + Git 2.2.0-2.4.0, maybe, or some other issue)

            I'm not saying that changes your need or desire to see this change get implemented. But it seems like there's likely more going on in your case if you need such massive timeouts.

            Best regards,
            Bryan Turner
            Atlassian Stash

            Bryan Turner (Inactive) added a comment - - edited Gordon, Do you actually have pull requests that take 30 minutes to complete a merge? Because I think the reality is that regardless of whether the UI waits for that or not, there's zero chance the human setting at their browser's going to wait that long. If you need the timeouts to be 6 or 7 times the default value I'd suggest opening a support ticket and working with us to determine why your merges are so slow. (NFS + Git 2.2.0-2.4.0, maybe, or some other issue) I'm not saying that changes your need or desire to see this change get implemented. But it seems like there's likely more going on in your case if you need such massive timeouts. Best regards, Bryan Turner Atlassian Stash

            I've just had 2 days of debugging due to the relationship of this error and config like this:

            plugin.stash-scm-git.pullrequest.merge.auto.timeout=900
            plugin.stash-scm-git.pullrequest.merge.real.timeout=1800

            This gives a feeling of completely confusing UI and multiple bug reports when a log running PR is going on and the UI is misleading as to its success - the PR creation succeeds and the merge also succeeds but the UI regularly reports errors and the users are tempted to push multiple "merge" buttons getting then "merge in progress" UI errors too.

            Would like to see this more of a bug than a suggestion as the front and backend have incompatible/non-complementary config.

            Gordon Waddell added a comment - I've just had 2 days of debugging due to the relationship of this error and config like this: plugin.stash-scm-git.pullrequest.merge.auto.timeout=900 plugin.stash-scm-git.pullrequest.merge.real.timeout=1800 This gives a feeling of completely confusing UI and multiple bug reports when a log running PR is going on and the UI is misleading as to its success - the PR creation succeeds and the merge also succeeds but the UI regularly reports errors and the users are tempted to push multiple "merge" buttons getting then "merge in progress" UI errors too. Would like to see this more of a bug than a suggestion as the front and backend have incompatible/non-complementary config.

              fhaehnel Felix (Inactive)
              bstuart Ben Stuart (Inactive)
              Votes:
              11 Vote for this issue
              Watchers:
              18 Start watching this issue

                Created:
                Updated:
                Resolved: