[minor] Updating code comments for pools #294

stackvana · Nov 1, 2017 · 3a9a3dc · 3a9a3dc
1 parent 5c170e2
commit 3a9a3dc
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 25 deletions.
diff --git a/lib/load-balancer/index.js b/lib/load-balancer/index.js
@@ -248,22 +248,6 @@ server.start = function start (opts, cb) {
         hookHandler(req, res);
       });
 
-      /*
-      var middle = oysMiddle({
-        secrets: secrets,
-        //prefix: "/files/api/v1/fs",
-        //checkRoleAccess: checkRoleAccess,
-        //parent: app.view,
-        //unauthorizedRoleAccess: config.messages.unauthorizedRoleAccess
-      });
-
-      app.use('/_oys', function(req, res, next){
-        middle(req, res, function(){
-          // console.log('oysMiddle callback');
-          // next callback is not needed as response should have ended
-        });
-      });
-      */
       app.use(server.handle404);
       updateBalancingTable(function (err) {
         // setTimeout to update the balancing table every 20 seconds
@@ -295,10 +279,12 @@ function updateBalancingTable (cb) {
     }
     // console.log('got pools.web', webs.length);
     if (webs !== null && typeof webs === "object") {
+      // if the pool is empty, simply assign the current webs
       if (config.pools.web.length === 0) {
         config.pools.web = webs;
       } else {
-        // for every existing node
+        // if the pool already contains web instances,
+        // for every existing web node...
         config.pools.web.forEach(function(oldWeb, i){
           var found = false;
           // check to see if it exists in the incoming table
@@ -311,11 +297,11 @@ function updateBalancingTable (cb) {
           });
           if (!found) {
             // if we didn't find a match, assume it's expired and remove it
-            console.log('performing web splice', i)
+            console.log('remove web node from pool', oldWeb);
             config.pools.web.splice(i, 1);
           }
         });
-        // for every incoming web
+        // for every incoming web node
         webs.forEach(function(newWeb){
           var found = false;
           // check against all existing webs
@@ -360,7 +346,7 @@ function updateBalancingTable (cb) {
           });
           if (!found) {
             // if we didn't find a match, assume it's expired and remove it
-            console.log('performing worker splice', i)
+            console.log('remove worker node from pool', oldWorker);
             config.pools.worker.splice(i, 1);
             //config.pools.worker.splice(i, 0);
           }

diff --git a/lib/web/index.js b/lib/web/index.js
@@ -574,23 +574,29 @@ server.start = function start (opts, cb) {
             pid: process.pid
           };
 
-          // check and remove any duplicate entries
-          // currently taking in entire set into memory, but should be okay since set contains < 100 members ( close to 10 )
+          // Register this web node into the web pool for load-balancer
+          // Note: Currently taking in entire set into memory, but should be okay since set contains < 100 members ( close to 10 )
           // TODO: move into node resource
+
+          // for each web node in the existing pool
           cache.smembers('pools.web', function (err, members) {
             if (err) {
               return cb(err)
             }
             var found = null;
-            members.forEach(function(member){
+            // check to see if a match was found based on host and port
+            members.forEach(function (member) {
               if (member.host === _node.host && member.port === _node.port) {
                 found = member;
               }
             });
+
+            // if a matching web node was not found, add it to the pool
             if (found === null) {
               addNode();
             } else {
-             // remove the node then add it
+              // if a matching node was found, remove it from the pool and add a fresh one
+              // Note: Removing the old node and adding a new one here will update the nodes metadata like pid and spawn time
               cache.srem('pools.web', found, function (err, rem) {
                 if (err) {
                   console.log(err);
@@ -602,11 +608,12 @@ server.start = function start (opts, cb) {
 
           function addNode () {
             console.log('registering with load balancer. adding to pools.web', _node);
-            // update pool with new server instance
+            // update web pool with new server instance
             cache.sadd('pools.web', _node, function (err, pools) {
               if (err) {
                 return cb(err)
               }
+              // now immediately update the balancing table
               updateBalancingTable(function (err) {
                 // setTimeout to update the balancing table every 20 seconds
                 loopUpdates();
@@ -636,6 +643,11 @@ function loopUpdates () {
   }, 1000);
 };
 
+//
+// Remark: Currently web nodes seem to require access to the worker pool
+//         This doesn't seem right because the load balancer should be responsible for routing requests to the /gateway
+// TODO:   See if we can remove this code and move gateway routes to load balancer instead
+//
 function updateBalancingTable (cb) {
   cache.smembers('pools.worker', function (err, workers) {
      if (err) {