Repository: vantage6/vantage6
Issue: [Bug] Node status ping/pong doesn't work when deployed

**Describe the bug**
Recently, we changed the way the node status is set in the database to a ping/pong strategy where the server sends a ping to all nodes once a minute, and puts those that respond to status online (see #40). However, when deployed, this does not seem to work:

```
2023-02-14T13:08:38.424387766Z Unknown callback received, ignoring.
2023-02-14T13:08:38.424429367Z 2023-02-14 13:08:38 - socketio.server - WARNING  - Unknown callback received, ignoring.
2023-02-14T13:08:38.427137205Z Unknown callback received, ignoring.
2023-02-14T13:08:38.427245506Z 2023-02-14 13:08:38 - socketio.server - WARNING  - Unknown callback received, ignoring.
2023-02-14T13:08:38.427676412Z Unknown callback received, ignoring.
2023-02-14T13:08:38.427790114Z 2023-02-14 13:08:38 - socketio.server - WARNING  - Unknown callback received, ignoring.
```

It might have to do with the multiple workers for the petronas server, but that's just a thought

Changed files:
vantage6-server/vantage6/server/__init__.py, vantage6-node/vantage6/node/socket.py, vantage6-server/vantage6/server/globals.py, vantage6-node/vantage6/node/__init__.py, vantage6-common/vantage6/common/globals.py, vantage6-server/vantage6/server/websockets.py

diff --git a/vantage6-common/vantage6/common/globals.py b/vantage6-common/vantage6/common/globals.py
index 755e0bfc..cd6591cb 100644
--- a/vantage6-common/vantage6/common/globals.py
+++ b/vantage6-common/vantage6/common/globals.py
@@ -26,3 +26,5 @@ PACKAGE_FOLDER = Path(__file__).parent.parent.parent
 VPN_CONFIG_FILE = 'vpn-config.ovpn.conf'
 
 DATABASE_TYPES = ["csv", "parquet", "sql", "sparql", "omop", "excel", "other"]
+
+PING_INTERVAL_SECONDS = 60
diff --git a/vantage6-node/vantage6/node/__init__.py b/vantage6-node/vantage6/node/__init__.py
index 681a175a..fef6e700 100644
--- a/vantage6-node/vantage6/node/__init__.py
+++ b/vantage6-node/vantage6/node/__init__.py
@@ -43,7 +43,7 @@ from enum import Enum
 from vantage6.common.docker.addons import (
     ContainerKillListener, check_docker_running, running_in_docker
 )
-from vantage6.common.globals import VPN_CONFIG_FILE
+from vantage6.common.globals import VPN_CONFIG_FILE, PING_INTERVAL_SECONDS
 from vantage6.common.exceptions import AuthenticationException
 from vantage6.common.docker.network_manager import NetworkManager
 from vantage6.common.task_status import TaskStatus
@@ -299,7 +299,7 @@ class Node(object):
         )
 
         # save task status to the server
-        update={'status': task_status}
+        update = {'status': task_status}
         if task_status == TaskStatus.NOT_ALLOWED:
             # set finished_at to now, so that the task is not picked up again
             # (as the task is not started at all, unlike other crashes, it will
@@ -798,6 +798,23 @@ class Node(object):
         self.log.info(f'Connected to host={self.server_io.host} on port='
                       f'{self.server_io.port}')
 
+        self.log.debug("Starting thread for to ping the server to notify this"
+                       " node is online.")
+        self.socketIO.start_background_task(self.__socket_ping_worker)
+
+    def __socket_ping_worker(self) -> None:
+        """
+        Send ping messages periodically to the server over the socketIO
+        connection to notify the server that this node is online
+        """
+        while True:
+            try:
+                self.socketIO.emit('ping', namespace='/tasks')
+            except Exception:
+                self.log.exception('Ping thread had an exception')
+            # Wait before sending next ping
+            time.sleep(PING_INTERVAL_SECONDS)
+
     def get_task_and_add_to_queue(self, task_id: int) -> None:
         """
         Fetches (open) task with task_id from the server. The `task_id` is
diff --git a/vantage6-node/vantage6/node/socket.py b/vantage6-node/vantage6/node/socket.py
index cd790c86..9918bb00 100644
--- a/vantage6-node/vantage6/node/socket.py
+++ b/vantage6-node/vantage6/node/socket.py
@@ -7,6 +7,7 @@ from vantage6.common.task_status import TaskStatus, has_task_failed
 from vantage6.node.util import logger_name
 
 
+
 class NodeTaskNamespace(ClientNamespace):
     """Class that handles incoming websocket events."""
 
@@ -42,17 +43,6 @@ class NodeTaskNamespace(ClientNamespace):
         # self.node_worker_ref.socketIO.disconnect()
         self.log.info('Disconnected from the server')
 
-    def on_ping(self) -> int:
-        """
-        Actions to be taken when server pings the nodes
-
-        Returns
-        -------
-        int
-            The id of this node so that server knows it is online
-        """
-        return self.node_worker_ref.server_io.whoami.id_
-
     def on_new_task(self, task_id: int):
         """
         Actions to be taken when node is notified of new task by server
diff --git a/vantage6-server/vantage6/server/__init__.py b/vantage6-server/vantage6/server/__init__.py
index 9e8dedce..eeab0d2c 100644
--- a/vantage6-server/vantage6/server/__init__.py
+++ b/vantage6-server/vantage6/server/__init__.py
@@ -35,12 +35,13 @@ from flask_principal import Principal, Identity, identity_changed
 from flask_socketio import SocketIO
 from threading import Thread
 
+from vantage6.common import logger_name
+from vantage6.common.globals import PING_INTERVAL_SECONDS
 from vantage6.server import db
 from vantage6.cli.context import ServerContext
 from vantage6.cli.globals import DEFAULT_SERVER_ENVIRONMENT
 from vantage6.server.model.base import DatabaseSessionManager, Database
 from vantage6.server.resource.common._schema import HATEOASModelSchema
-from vantage6.common import logger_name
 from vantage6.server.permission import RuleNeed, PermissionManager
 from vantage6.server.globals import (
     APPNAME,
@@ -50,7 +51,6 @@ from vantage6.server.globals import (
     SUPER_USER_INFO,
     REFRESH_TOKENS_EXPIRE_HOURS,
     DEFAULT_SUPPORT_EMAIL_ADDRESS,
-    MAX_RESPONSE_TIME_PING,
     MIN_TOKEN_VALIDITY_SECONDS,
     MIN_REFRESH_TOKEN_EXPIRY_DELTA,
 )
@@ -121,9 +121,9 @@ class ServerApp:
         self.__version__ = __version__
 
         # set up socket ping/pong
-        log.debug(
-            "Starting thread for socket ping/pong between server and nodes")
-        self.socketio.start_background_task(self.__socket_pingpong_worker)
+        log.debug("Starting thread to set node status")
+        t = Thread(target=self.__node_status_worker, daemon=True)
+        t.start()
 
         log.info("Initialization done")
 
@@ -555,51 +555,33 @@ class ServerApp:
             user.save()
         return self
 
-    def __socket_pingpong_worker(self) -> None:
+    def __node_status_worker(self) -> None:
         """
-        Send ping messages periodically to nodes over the socketIO connection
-        and set node status online/offline depending on whether they respond
-        or not.
+        Set node status to offline if they haven't send a ping message in a
+        while.
         """
-        # when starting up the server, wait a few seconds to allow nodes that
-        # are already online to connect back to the server (otherwise they
-        # would be incorrectly set to offline for one period)
-        time.sleep(5)
-
         # start periodic check if nodes are responsive
         while True:
             # Send ping event
             try:
-                ping_time = dt.datetime.utcnow()
-                self.socketio.emit(
-                    'ping', namespace='/tasks', room='all_nodes',
-                    callback=self.__pong_response
-                )
+                before_wait = dt.datetime.utcnow()
 
-                # Wait a while to give nodes opportunity to pong
-                time.sleep(MAX_RESPONSE_TIME_PING)
+                # Wait a while to give nodes opportunity to pong. This interval
+                # is a bit longer than the interval at which the nodes ping,
+                # because we want to make sure that the nodes have had time to
+                # respond.
+                time.sleep(PING_INTERVAL_SECONDS + 5)
 
                 # Check for each node that is online if they have responded.
                 # Otherwise set them to offline.
                 online_status_nodes = db.Node.get_online_nodes()
                 for node in online_status_nodes:
-                    if node.last_seen < ping_time:
+                    if node.last_seen < before_wait:
                         node.status = 'offline'
                         node.save()
-
-                # we need to sleep here for a bit to make sure that there is a
-                # delay between setting nodes offline and pinging again - this
-                # prevents a racing condition in setting status
-                time.sleep(5)
             except Exception:
-                log.exception('Pingpong thread had an exception')
-                time.sleep(MAX_RESPONSE_TIME_PING)
-
-    def __pong_response(self, node_id) -> None:
-        node = db.Node.get(node_id)
-        node.status = 'online'
-        node.last_seen = dt.datetime.utcnow()
-        node.save()
+                log.exception('Node-status thread had an exception')
+                time.sleep(PING_INTERVAL_SECONDS)
 
 
 def run_server(config: str, environment: str = DEFAULT_SERVER_ENVIRONMENT,
diff --git a/vantage6-server/vantage6/server/globals.py b/vantage6-server/vantage6/server/globals.py
index 5ea9b82e..38565dd9 100644
--- a/vantage6-server/vantage6/server/globals.py
+++ b/vantage6-server/vantage6/server/globals.py
@@ -50,6 +50,3 @@ DEFAULT_SUPPORT_EMAIL_ADDRESS = 'support@vantage6.ai'
 
 # default time that token is valid in minutes
 DEFAULT_EMAILED_TOKEN_VALIDITY_MINUTES = 60
-
-# maximum time given to nodes to respond to ping in seconds
-MAX_RESPONSE_TIME_PING = 60
diff --git a/vantage6-server/vantage6/server/websockets.py b/vantage6-server/vantage6/server/websockets.py
index bbdd682b..f1fb8a80 100644
--- a/vantage6-server/vantage6/server/websockets.py
+++ b/vantage6-server/vantage6/server/websockets.py
@@ -1,6 +1,7 @@
 import logging
 from typing import Dict
 import jwt
+import datetime as dt
 
 from flask import request, session
 from flask_jwt_extended import get_jwt_identity, verify_jwt_in_request
@@ -296,6 +297,16 @@ class DefaultSocketNamespace(Namespace):
         # cleanup (e.g. database session)
         self.__cleanup()
 
+    def on_ping(self) -> None:
+        """
+        A client sends a ping to the server. The server detects who sent the
+        ping and sets them as online.
+        """
+        auth = db.Authenticatable.get(session.auth_id)
+        auth.status = 'online'
+        auth.last_seen = dt.datetime.utcnow()
+        auth.save()
+
     def __join_room_and_notify(self, room: str) -> None:
         """
         Joins room and notify other clients in this room.