add config.performance.unfurlSrvCacheTtl option for caching resolveSrv calls

config.json

@@ -10,7 +10,8 @@
 		"connectTimeout": 2500
 	},
 	"performance": {
-		"skipUnfurlSrv": true
+		"skipUnfurlSrv": false,
+		"unfurlSrvCacheTtl": 120000
 	},
 	"logToDatabase": true,
 	"graphDuration": 86400000,

docs/CHANGELOG.md

@@ -1,8 +1,9 @@
 **5.4.0** *(May 8 2020)*
 - Favicons are now served over the http server (using a unique hash). This allows the favicons to be safely cached for long durations and still support dynamic updates.
 - Adds "graphDurationLabel" to `config.json` which allows you to manually modify the "24h Peak" label to a custom time duration.
-- Adds "serverGraphDuration" to `config.json` which allows you to specify the max time duration for the individual server player count graphs.
-- Adds "performance.skipUnfurlSrv" to `config.json` which allows you to skip SRV unfurling when pinging. For those who aren't pinging servers that use SRV records, this should help speed up ping times.
+- Adds "serverGraphDuration" (default 3 minutes) to `config.json` which allows you to specify the max time duration for the individual server player count graphs.
+- Adds "performance.skipUnfurlSrv" (default false) to `config.json` which allows you to skip SRV unfurling when pinging. For those who aren't pinging servers that use SRV records, this should help speed up ping times.
+- Adds "performance.skipUnfurlSrv" (default 120 seconds) to `config.json` which allows you specify how long a SRV unfurl should be cached for. This prevents repeated, potentially slow lookups. Set to 0 to disable caching.
 - Ping timestamps are now shared between all server pings. This means less data transfer when loading or updating the page, less memory usage by the backend and frontend, and less hectic updates on the frontend.
 - Optimized several protocol level schemas to remove legacy format waste. Less bandwidth!
 - Fixes a bug where favicons may not be updated if the page is loaded prior to their initialization.

lib/ping.js

@@ -1,5 +1,3 @@
-const dns = require('dns')
-
 const minecraftJavaPing = require('mc-ping-updated')
 const minecraftBedrockPing = require('mcpe-ping-fixed')
 
@@ -8,10 +6,10 @@ const MessageOf = require('./message')
 
 const config = require('../config')
 
-function ping (host, port, type, timeout, callback, version) {
-  switch (type) {
+function ping (serverRegistration, timeout, callback, version) {
+  switch (serverRegistration.data.type) {
     case 'PC':
-      unfurlSrv(host, port, (host, port) => {
+      serverRegistration.unfurlSrv((host, port) => {
         minecraftJavaPing(host, port || 25565, (err, res) => {
           if (err) {
             callback(err)
@@ -35,13 +33,13 @@ function ping (host, port, type, timeout, callback, version) {
       break
 
     case 'PE':
-      minecraftBedrockPing(host, port || 19132, (err, res) => {
+      minecraftBedrockPing(serverRegistration.data.ip, serverRegistration.data.port || 19132, (err, res) => {
         if (err) {
           callback(err)
         } else {
           callback(null, {
             players: {
-              online: capPlayerCount(host, parseInt(res.currentPlayers))
+              online: capPlayerCount(serverRegistration.data.ip, parseInt(res.currentPlayers))
             }
           })
         }
@@ -49,25 +47,10 @@ function ping (host, port, type, timeout, callback, version) {
       break
 
     default:
-      throw new Error('Unsupported type: ' + type)
+      throw new Error('Unsupported type: ' + serverRegistration.data.type)
   }
 }
 
-function unfurlSrv (hostname, port, callback) {
-  if (config.performance && config.performance.skipUnfurlSrv) {
-    callback(hostname, port)
-    return
-  }
-
-  dns.resolveSrv('_minecraft._tcp.' + hostname, (_, records) => {
-    if (!records || records.length < 1) {
-      callback(hostname, port)
-    } else {
-      callback(records[0].name, records[0].port)
-    }
-  })
-}
-
 // player count can be up to 1^32-1, which is a massive scale and destroys browser performance when rendering graphs
 // Artificially cap and warn to prevent propogating garbage
 function capPlayerCount (host, playerCount) {
@@ -136,7 +119,7 @@ class PingController {
     for (const serverRegistration of this._app.serverRegistrations) {
       const version = serverRegistration.getNextProtocolVersion()
 
-      ping(serverRegistration.data.ip, serverRegistration.data.port, serverRegistration.data.type, config.rates.connectTimeout, (err, resp) => {
+      ping(serverRegistration, config.rates.connectTimeout, (err, resp) => {
         if (err) {
           logger.log('error', 'Failed to ping %s: %s', serverRegistration.data.ip, err.message)
         }

lib/servers.js

@@ -1,4 +1,5 @@
 const crypto = require('crypto')
+const dns = require('dns')
 
 const TimeTracker = require('./time')
 const Server = require('./server')
@@ -313,6 +314,62 @@ class ServerRegistration {
       color: this.data.color
     }
   }
+
+  unfurlSrv (callback) {
+    // Skip unfurling SRV, instantly return pre-configured data
+    if (config.performance && config.performance.skipUnfurlSrv) {
+      callback(this.data.ip, this.data.port)
+      return
+    }
+
+    const timestamp = new Date().getTime()
+
+    // If a cached copy exists and is within its TTL, instantly return
+    if (this._lastUnfurlSrv && timestamp - this._lastUnfurlSrv.timestamp <= config.performance.unfurlSrvCacheTtl) {
+      callback(this._lastUnfurlSrv.ip, this._lastUnfurlSrv.port)
+      return
+    }
+
+    // Group callbacks into an array
+    // Once resolved, fire callbacks sequentially
+    // This avoids callbacks possibly executing out of order
+    if (!this._unfurlSrvCallbackQueue) {
+      this._unfurlSrvCallbackQueue = []
+    }
+
+    this._unfurlSrvCallbackQueue.push(callback)
+
+    // Prevent multiple #resolveSrv calls per ServerRegistration
+    if (!this._isUnfurlingSrv) {
+      this._isUnfurlingSrv = true
+
+      dns.resolveSrv('_minecraft._tcp' + this.data.ip, (_, records) => {
+        this._lastUnfurlSrv = {
+          timestamp
+        }
+
+        if (records && records.length > 0) {
+          this._lastUnfurlSrv.ip = records[0].name
+          this._lastUnfurlSrv.port = records[0].port
+        } else {
+          // Provide fallback to pre-configured data
+          this._lastUnfurlSrv.ip = this.data.ip
+          this._lastUnfurlSrv.port = this.data.port
+        }
+
+        // Fire the waiting callbacks in queue
+        // Release blocking flag to allow new #resolveSrv calls
+        this._isUnfurlingSrv = false
+
+        for (const callback of this._unfurlSrvCallbackQueue) {
+          callback(this._lastUnfurlSrv.ip, this._lastUnfurlSrv.port)
+        }
+
+        // Reset the callback queue
+        this._unfurlSrvCallbackQueue = []
+      })
+    }
+  }
 }
 
 module.exports = ServerRegistration

main.js

@@ -34,6 +34,11 @@ if (config.performance && config.performance.skipUnfurlSrv) {
   logger.log('warn', '"performance.skipUnfurlSrv" is enabled. Any configured hosts using SRV records may not properly resolve.')
 }
 
+if (!config.performance || typeof config.performance.unfurlSrvCacheTtl === 'undefined') {
+  logger.log('warn', '"performance.unfurlSrvCacheTtl" is not defined in config.json - defaulting to 120 seconds!')
+  config.performance.unfurlSrvCacheTtl = 2 * 60 * 1000
+}
+
 if (!config.logToDatabase) {
   logger.log('warn', 'Database logging is not enabled. You can enable it by setting "logToDatabase" to true in config.json. This requires sqlite3 to be installed.')