AMBARI-286. Make TxnProgressWidget Immune To Re-Fetch Race Conditions (Contributed by Varun)

git-svn-id: https://svn.apache.org/repos/asf/incubator/ambari/branches/ambari-186@1347030 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -6,6 +6,8 @@ characters wide.
 
 Release 0.1.x - unreleased
 
+  AMBARI-286. Make TxnProgressWidget Immune To Re-Fetch Race Conditions (Varun via Vikram)
+
   AMBARI-526. Display client nodes as part of cluster topology display. (Varun via Vikram)
 
   AMBARI-359. invalid parameter java_needed during uninstall (Ramya via Vikram)

hmc/js/txnUtils.js

@@ -60,12 +60,16 @@ function TxnProgressWidget( txnProgressContext, txnProgressStatusMessage, txnPro
   };
 
   var pdpResponseHandler = {
-    success: function (e,pdp) {
+    success: function (e, pdp) {
 
       /* What we're here to render. */
       var txnProgressMarkup = 
         '<img id=txnProgressLoadingImgId class=loadingImg src=../images/loading.gif />'; 
 
+      var noNeedForFurtherPolling = false;
+      var txnProgressStatusDivContent = '';
+      var txnProgressStatusDivCssClass = '';
+
       var txnProgress = e.response.meta.progress;
 
       /* Guard against race conditions where txnProgress is null because the 
@@ -73,6 +77,16 @@ function TxnProgressWidget( txnProgressContext, txnProgressStatusMessage, txnPro
        */
       if (txnProgress) {
 
+        /* The first time we get back meaningful progress data, pause the 
+         * automatic polling to avoid race conditions where response N+1 
+         * is made (and returns with fresh data) while request N hasn't 
+         * yet been fully processed.
+         *
+         * We'll unpause at the end, after we've performed the rendering
+         * of the updated states.
+         */
+        pdp.pause();
+
         var txnProgressStates = txnProgress.subTxns || [];
         globalYui.log(globalYui.Lang.dump(txnProgressStates));
 
@@ -181,9 +195,32 @@ function TxnProgressWidget( txnProgressContext, txnProgressStatusMessage, txnPro
             ( presentTxnProgressState.description, 'txnProgressStatePending' );
         }
 
-        var noNeedForFurtherPolling = false;
-        var txnProgressStatusDivContent = '';
-        var txnProgressStatusDivCssClass = '';
+        txnProgressMarkup += '</ul>';
+
+        /* Make sure we have some progress data to show - if not, 
+         * we'll just show a loading image until this is non-null.
+         *
+         * The additional check for txnProgress.processRunning is to account 
+         * for cases where there are no subTxns (because it's all a no-op at 
+         * the backend) - the loading image should only be shown as long as 
+         * the backend is still working; after that, we should break out of
+         * the loading image loop and let the user know that there was
+         * nothing to be done.
+         */
+        if( txnProgress.subTxns == null ) {
+          if( txnProgress.processRunning == 0 ) {
+            txnProgressMarkup = 
+              '<br/>' + 
+              '<div class=txnNoOpMsg>' + 
+                'Nothing to do for this transaction; enjoy the freebie!' +
+              '</div>' + 
+              '<br/>';
+          } 
+          else {
+            txnProgressMarkup = 
+              '<img id=txnProgressLoadingImgId class=loadingImg src=../images/loading.gif />';
+          }
+        }
 
         /* We can break this polling cycle in one of 2 ways: 
          * 
@@ -211,61 +248,44 @@ function TxnProgressWidget( txnProgressContext, txnProgressStatusMessage, txnPro
           txnProgressStatusDivContent = this.txnProgressStatusMessage.failure;
           txnProgressStatusDivCssClass = 'statusError';
         }
+      }
+
+      /* Render txnProgressMarkup before making any decisions about the
+       * future state of pdp. 
+       */
+      globalYui.log('About to generate markup: ' + txnProgressMarkup);
+      globalYui.one('#txnProgressDynamicRenderDivId').setContent( txnProgressMarkup );
 
-        if( noNeedForFurtherPolling ) {
+      /* And before checking out, decide whether we're done with this txn 
+       * or whether any more polling is required.
+       */
+      if (noNeedForFurtherPolling) {
 
-          /* We've made all the progress we could have, so stop polling. */
-          pdp.stop();
+        /* We've made all the progress we could have, so stop polling. */
+        pdp.stop();
 
-          var txnProgressStatusDiv = globalYui.one('#txnProgressStatusDivId');
-          
-          txnProgressStatusDiv.addClass(txnProgressStatusDivCssClass);
-          txnProgressStatusDiv.one('#txnProgressStatusMessageDivId').setContent(txnProgressStatusDivContent);
-          txnProgressStatusDiv.setStyle('display', 'block');
+        var txnProgressStatusDiv = globalYui.one('#txnProgressStatusDivId');
+        
+        txnProgressStatusDiv.addClass(txnProgressStatusDivCssClass);
+        txnProgressStatusDiv.one('#txnProgressStatusMessageDivId').setContent(txnProgressStatusDivContent);
+        txnProgressStatusDiv.setStyle('display', 'block');
 
-          /* Run the post-completion fixups. */
-          if( txnProgressStatusDivCssClass == 'statusOk' ) {
-            if( this.txnProgressPostCompletionFixup.success ) {
-              this.txnProgressPostCompletionFixup.success(this);
-            }
-          }
-          else if( txnProgressStatusDivCssClass == 'statusError' ) {
-            if( this.txnProgressPostCompletionFixup.failure ) {
-              this.txnProgressPostCompletionFixup.failure(this);
-            }
+        /* Run the post-completion fixups. */
+        if (txnProgressStatusDivCssClass == 'statusOk') {
+          if (this.txnProgressPostCompletionFixup.success) {
+            this.txnProgressPostCompletionFixup.success(this);
           }
         }
-
-        txnProgressMarkup += '</ul>';
-
-        /* Make sure we have some progress data to show - if not, 
-         * we'll just show a loading image until this is non-null.
-         *
-         * The additional check for txnProgress.processRunning is to account 
-         * for cases where there are no subTxns (because it's all a no-op at 
-         * the backend) - the loading image should only be shown as long as 
-         * the backend is still working; after that, we should break out of
-         * the loading image loop and let the user know that there was
-         * nothing to be done.
-         */
-        if( txnProgress.subTxns == null ) {
-          if( txnProgress.processRunning == 0 ) {
-            txnProgressMarkup = 
-              '<br/>' + 
-              '<div class=txnNoOpMsg>' + 
-                'Nothing to do for this transaction; enjoy the freebie!' +
-              '</div>' + 
-              '<br/>';
-          } 
-          else {
-            txnProgressMarkup = 
-              '<img id=txnProgressLoadingImgId class=loadingImg src=../images/loading.gif />';
+        else if (txnProgressStatusDivCssClass == 'statusError') {
+          if (this.txnProgressPostCompletionFixup.failure) {
+            this.txnProgressPostCompletionFixup.failure(this);
           }
         }
       }
-
-      globalYui.log('About to generate markup: ' + txnProgressMarkup);
-      globalYui.one('#txnProgressDynamicRenderDivId').setContent( txnProgressMarkup );
+      else {
+        /* There's still more progress to be made, so unpause. */
+        pdp.unPause();
+      }
 
     }.bind(this),
 

hmc/js/utils.js

@@ -350,6 +350,9 @@ function PeriodicDataPoller( dataSourceContext, responseHandler ) {
 
   this.responseHandler = responseHandler;
 
+  /* Of course, we're not paused when we start off. */
+  this.paused = false;
+
   this.dataSource = new globalYui.DataSource.IO ({
     source: this.dataSourceContext.source
   });
@@ -367,28 +370,37 @@ function PeriodicDataPoller( dataSourceContext, responseHandler ) {
     request: this.dataSourceContext.request,
     callback: {
       success: function (e) {
-        /* Reset our failure count every time we succeed. */
-        this.dataSourcePollFailureCount = 0;
 
-        /* Invoke user-pluggable code. */
-        if( this.responseHandler.success ) {
-          this.responseHandler.success( e, this );
-        }
+        /* Avoid race conditions in JS by not processing incoming responses 
+         * from the backend if the PDP is paused (which is our signal that
+         * a previous response is still in the middle of being processed). 
+         */
+        if( !(this.isPaused()) ) {
+          /* Reset our failure count every time we succeed. */
+          this.dataSourcePollFailureCount = 0;
 
+          /* Invoke user-pluggable code. */
+          if( this.responseHandler.success ) {
+            this.responseHandler.success( e, this );
+          }
+        }
       }.bind(this),
 
       failure: function (e) {
-        ++this.dataSourcePollFailureCount;
 
-        if( this.dataSourcePollFailureCount > this.dataSourceContext.maxFailedAttempts ) {
+        if( !(this.isPaused()) ) {
+          ++this.dataSourcePollFailureCount;
 
-          /* Invoke user-pluggable code. */
-          if( this.responseHandler.failure ) {
-            this.responseHandler.failure( e, this );
-          }
+          if( this.dataSourcePollFailureCount > this.dataSourceContext.maxFailedAttempts ) {
 
-          /* No point making any more attempts. */
-          this.stop();
+            /* Invoke user-pluggable code. */
+            if( this.responseHandler.failure ) {
+              this.responseHandler.failure( e, this );
+            }
+
+            /* No point making any more attempts. */
+            this.stop();
+          }
         }
       }.bind(this)
     }
@@ -398,15 +410,53 @@ function PeriodicDataPoller( dataSourceContext, responseHandler ) {
 /* Start polling. */
 PeriodicDataPoller.prototype.start = function() {
 
-  this.dataSourcePollHandle = this.dataSource.setInterval( this.dataSourceContext.pollInterval, this.dataSourcePollRequestContext );
+  this.dataSourcePollHandle = this.dataSource.setInterval
+    ( this.dataSourceContext.pollInterval, this.dataSourcePollRequestContext );
 }
 
 /* Stop polling. */
 PeriodicDataPoller.prototype.stop = function() {
 
+  /* Always unPause() during stop(), so the next start() won't be neutered. */
+  this.unPause();
   this.dataSource.clearInterval( this.dataSourcePollHandle );
 }
 
+/* When the PDP is paused, the polling continues on its regular fixed 
+ * interval, but this.responseHandler is not invoked, thus avoiding
+ * a race condition (at least) in JS.
+ *
+ * TODO XXX Improve upon this to not even make calls to the backend
+ * while not losing our periodicity.
+ */
+PeriodicDataPoller.prototype.pause = function() {
+
+  this.paused = true;
+}
+
+PeriodicDataPoller.prototype.unPause = function() {
+
+  this.paused = false;
+}
+
+PeriodicDataPoller.prototype.isPaused = function() {
+
+  return this.paused;
+}
+
+/* Perform a one-time poll. 
+ *
+ * Meant to be used when the polling is not at a set frequency (as with the 
+ * start()/stop() pair), and is instead meant to be under explicit 
+ * control of the application. 
+ */
+PeriodicDataPoller.prototype.pollOnce = function() {
+
+  globalYui.io(this.dataSourceContext.source + this.dataSourcePollRequestContext.request, {
+    on: this.dataSourcePollRequestContext.callback
+  });
+}
+
 function titleCase(word){
   return word.substr(0,1).toUpperCase() + word.substr(1).toLowerCase();
 }