Re: Review Request 51677: AMBARI-18327: multiple clicks on "Next" button of Step-4 (Choose Services) causes skipping of steps while installing a cluster
> On Sept. 7, 2016, 4:56 a.m., Alexandr Antonenko wrote:
> > In step-4 the flag "nextBtnClickInProgress" was not set anywhere -> We set
> > nextBtnClickInProgress in router for step 4
> >
> > So if a timeout was manually added in the "submit" function of the
> > controller, we could allow multiple clicks to be registered -> I think this
> > is not the right way to test and create bugs based on such synthetic test
> > case. Because if I will wrap with timeout your newly added line where you
> > set nextBtnClickInProgress to true, results will be the same, I will be
> > able to perform multiple clicks.
> >
> > I was not able to reproduce this issue manually on latest trunk.
> >
> > This behavior is reproducible only when the host is slow, which would allow
> > enough time to click the Next button multiple times -> Even when I set
> > throttling to 50 kb/s and delay 500ms (the slowest setting in chrome
> > network tab) I'm unable to reproduce
> >
> > I think what we have in router for step 4 is enough to prevent multiple
> > clicks
>
> Keta Patel wrote:
> Hello Alexandr,
> The "nextBtnClickInProgress" flag is set in the router when we call the
> App.router.send('next') from the controller. This misses out the case when
> the point of execution is still in the controller validating dependencies
> with a callback function. Due to the presence of this callback, there is a
> possibility for context switch and thus, allowing the registered multiple
> clicks to execute the same "submit()" function in the controller.
>
> The addition of the timeout was only to simulate the lag from the host
> machine in performing the validating functions like service dependency checks
> or some other validation functions in "validate()".
>
> This issue is difficult to reproduce and depends heavily on how fast the
> host machine responds back (It is not related to the browser because the
> multiple clicks, when registered, do succeed in executing the "submit()"
> function those many times and hence skipping multiple steps via the
> App.router.send('next') call.)
>
> This is my understanding of why and how the issue is still present, but I
> have also not been able to reproduce this behavior on the latest trunk. I had
> to revise the fix for an earlier branch (where I was able to reproduce it
> only once) when I realized that the fix is not perfect.
I was not talking about browser I was talking about chrome dev tools, please
check
https://developers.google.com/web/tools/chrome-devtools/profile/network-performance/network-conditions
This is a tool to simulate that what you call "host lag"
validate is not an deferred method, it is straightforward js function that will
be executed line by line, so it cannot actually depend on host. It is totally
executed on client side in browser. It executes very fast ) no need to block
button for milliseconds to execute validate function.
And once again your test case is syntetic, it's not real life case. I can take
your patch and put timeout right before you set
"App.set('router.nextBtnClickInProgress', true);" and will have the behavior as
your are talking about.
- Alexandr
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51677/#review147972
---
On Sept. 6, 2016, 10:41 p.m., Keta Patel wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51677/
> ---
>
> (Updated Sept. 6, 2016, 10:41 p.m.)
>
>
> Review request for Ambari, Alexandr Antonenko and Di Li.
>
>
> Bugs: AMBARI-18327
> https://issues.apache.org/jira/browse/AMBARI-18327
>
>
> Repository: ambari
>
>
> Description
> ---
>
> While installing a cluster if the Ambari user happened to click the "Next"
> button of Step-4 (Choose Services step) more than once, then it could lead to
> skipping directly to step-6 or 7 or 8 depending on the number of times the
> click got registered. This behavior is reproducible only when the host is
> slow, which would allow enough time to click the Next button multiple times.
>
> This issue is related to AMBARI-14574, and is a revised fix for the Step-4.
>
>
> Diffs
> -
>
> ambari-web/app/controllers/wizard/step4_controller.js 04e6784
>
> Diff: https://reviews.apache.org/r/51677/diff/
>
>
> Testing
> ---
>
> **FIX:**
> In step-4 the flag "nextBtnClickInProgress" was not set anywhere. So if a
> timeout was manually added in the "submit" function of the controller, we
> could allow multiple clicks to be registered and these would be processed
> allowing steps to be skipped. In the revised fix, the flag
> "nextBtnClickInProgress" is set in the "submit" function.
>
Re: Review Request 51677: AMBARI-18327: multiple clicks on "Next" button of Step-4 (Choose Services) causes skipping of steps while installing a cluster
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51677/#review147972 --- In step-4 the flag "nextBtnClickInProgress" was not set anywhere -> We set nextBtnClickInProgress in router for step 4 So if a timeout was manually added in the "submit" function of the controller, we could allow multiple clicks to be registered -> I think this is not the right way to test and create bugs based on such synthetic test case. Because if I will wrap with timeout your newly added line where you set nextBtnClickInProgress to true, results will be the same, I will be able to perform multiple clicks. I was not able to reproduce this issue manually on latest trunk. This behavior is reproducible only when the host is slow, which would allow enough time to click the Next button multiple times -> Even when I set throttling to 50 kb/s and delay 500ms (the slowest setting in chrome network tab) I'm unable to reproduce I think what we have in router for step 4 is enough to prevent multiple clicks - Alexandr Antonenko On Sept. 6, 2016, 10:41 p.m., Keta Patel wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51677/ > --- > > (Updated Sept. 6, 2016, 10:41 p.m.) > > > Review request for Ambari, Alexandr Antonenko and Di Li. > > > Bugs: AMBARI-18327 > https://issues.apache.org/jira/browse/AMBARI-18327 > > > Repository: ambari > > > Description > --- > > While installing a cluster if the Ambari user happened to click the "Next" > button of Step-4 (Choose Services step) more than once, then it could lead to > skipping directly to step-6 or 7 or 8 depending on the number of times the > click got registered. This behavior is reproducible only when the host is > slow, which would allow enough time to click the Next button multiple times. > > This issue is related to AMBARI-14574, and is a revised fix for the Step-4. > > > Diffs > - > > ambari-web/app/controllers/wizard/step4_controller.js 04e6784 > > Diff: https://reviews.apache.org/r/51677/diff/ > > > Testing > --- > > **FIX:** > In step-4 the flag "nextBtnClickInProgress" was not set anywhere. So if a > timeout was manually added in the "submit" function of the controller, we > could allow multiple clicks to be registered and these would be processed > allowing steps to be skipped. In the revised fix, the flag > "nextBtnClickInProgress" is set in the "submit" function. > "nextBtnClickInProgress" causes "isSubmitDisabled" function also to return > "true" or "false" depending on whether it is "true" or "false" respectively. > For this reason, we also need to reset the "nextBtnClickInProgress" flag at > the end of "validate" function so that the subsequent valid "submit" calls, > on closing pop-up(s) can be processed. > > **TESTING:** > ambari-web unit tests affter applying the patch: > > 30182 tests complete (46 seconds) > 151 tests pending > > > Also, did manual testing by adding a timeout in the submit() of > step4_controller.js to allow multiple clicks to be registered on the Next > button. > > > Thanks, > > Keta Patel > >
Re: Review Request 51677: AMBARI-18327: multiple clicks on "Next" button of Step-4 (Choose Services) causes skipping of steps while installing a cluster
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51677/ --- (Updated Sept. 6, 2016, 10:41 p.m.) Review request for Ambari, Alexandr Antonenko and Di Li. Bugs: AMBARI-18327 https://issues.apache.org/jira/browse/AMBARI-18327 Repository: ambari Description --- While installing a cluster if the Ambari user happened to click the "Next" button of Step-4 (Choose Services step) more than once, then it could lead to skipping directly to step-6 or 7 or 8 depending on the number of times the click got registered. This behavior is reproducible only when the host is slow, which would allow enough time to click the Next button multiple times. This issue is related to AMBARI-14574, and is a revised fix for the Step-4. Diffs - ambari-web/app/controllers/wizard/step4_controller.js 04e6784 Diff: https://reviews.apache.org/r/51677/diff/ Testing (updated) --- **FIX:** In step-4 the flag "nextBtnClickInProgress" was not set anywhere. So if a timeout was manually added in the "submit" function of the controller, we could allow multiple clicks to be registered and these would be processed allowing steps to be skipped. In the revised fix, the flag "nextBtnClickInProgress" is set in the "submit" function. "nextBtnClickInProgress" causes "isSubmitDisabled" function also to return "true" or "false" depending on whether it is "true" or "false" respectively. For this reason, we also need to reset the "nextBtnClickInProgress" flag at the end of "validate" function so that the subsequent valid "submit" calls, on closing pop-up(s) can be processed. **TESTING:** ambari-web unit tests affter applying the patch: 30182 tests complete (46 seconds) 151 tests pending Also, did manual testing by adding a timeout in the submit() of step4_controller.js to allow multiple clicks to be registered on the Next button. Thanks, Keta Patel
Review Request 51677: AMBARI-18327: multiple clicks on "Next" button of Step-4 (Choose Services) causes skipping of steps while installing a cluster
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51677/ --- Review request for Ambari, Alexandr Antonenko and Di Li. Bugs: AMBARI-18327 https://issues.apache.org/jira/browse/AMBARI-18327 Repository: ambari Description --- While installing a cluster if the Ambari user happened to click the "Next" button of Step-4 (Choose Services step) more than once, then it could lead to skipping directly to step-6 or 7 or 8 depending on the number of times the click got registered. This behavior is reproducible only when the host is slow, which would allow enough time to click the Next button multiple times. This issue is related to AMBARI-14574, and is a revised fix for the Step-4. Diffs - ambari-web/app/controllers/wizard/step4_controller.js 04e6784 Diff: https://reviews.apache.org/r/51677/diff/ Testing --- ambari-web unit tests affter applying the patch: 30182 tests complete (46 seconds) 151 tests pending Also, did manual testing by adding a timeout in the submit() of step4_controller.js to allow multiple clicks to be registered on the Next button. Thanks, Keta Patel
