1223: Warn when web and database server time differ (#7815)

Author: labkey-nicka

api/src/org/labkey/api/data/dialect/SqlDialect.java

@@ -59,6 +59,7 @@
 import org.labkey.api.module.ModuleLoader;
 import org.labkey.api.query.FieldKey;
 import org.labkey.api.util.ExceptionUtil;
+import org.labkey.api.util.HtmlString;
 import org.labkey.api.util.MemTracker;
 import org.labkey.api.util.StringUtilsLabKey;
 import org.labkey.api.util.SystemMaintenance;
@@ -80,6 +81,8 @@
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.sql.Types;
+import java.time.Duration;
+import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -95,6 +98,7 @@
 import java.util.regex.Pattern;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
@@ -168,7 +172,7 @@ public String getOtherDatabaseThreads()
     {
         StringBuilder sb = new StringBuilder();
 
-        // Per 18789, also include threads without db connections.
+        // Per Issue 18789, also include threads without db connections.
         List<Thread> dbThreads = new ArrayList<>();
 
         for (Map.Entry<Thread, StackTraceElement[]> entry : Thread.getAllStackTraces().entrySet())
@@ -863,25 +867,25 @@ public SQLFragment getNumericCast(SQLFragment expression)
      * @param arguments Arguments passed from the LK SQL
      * @return the dialect equivalent SQLFragrment
      */
-    public SQLFragment getGreatestAndLeastSQL(String method, SQLFragment... arguments)
-    {
-        throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
-    }
-
-    public boolean supportsIsNumeric()
-    {
-        return false;
-    }
-
-    public SQLFragment isNumericExpr(SQLFragment expression)
-    {
-        throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
-    }
-
-    public void handleCreateDatabaseException(SQLException e) throws ServletException
-    {
-        throw(new ServletException("Can't create database", e));
-    }
+    public SQLFragment getGreatestAndLeastSQL(String method, SQLFragment... arguments)
+    {
+        throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
+    }
+
+    public boolean supportsIsNumeric()
+    {
+        return false;
+    }
+
+    public SQLFragment isNumericExpr(SQLFragment expression)
+    {
+        throw new UnsupportedOperationException(getClass().getSimpleName() + " does not implement");
+    }
+
+    public void handleCreateDatabaseException(SQLException e) throws ServletException
+    {
+        throw(new ServletException("Can't create database", e));
+    }
 
     /**
      * Wrap one or more INSERT statements to allow explicit specification
@@ -1990,12 +1994,61 @@ public final Collection<String> getExecutionPlan(DbScope scope, SQLFragment sql,
     // Add any database configuration warnings (e.g., missing aggregate function or deprecated database server version)
     // to display in the page header for administrators. This will be called:
     // - Only on the LabKey DataSource's dialect instance (not external data sources)
-    // - After the core module has been upgraded and the dialect has been prepared for the last time, meaning the dialect
+    // - After the core module has been upgraded, and the dialect has been prepared for the last time, meaning the dialect
     //   should reflect the final database configuration
     public void addAdminWarningMessages(Warnings warnings, boolean showAllWarnings)
     {
     }
 
+    public static final long TIME_DIFFERENCE_WARNING_SECONDS = 10;
+
+    public static ServerDatabaseTimeDifference getServerDatabaseTimeDifference(DbScope scope)
+    {
+        // Compare LocalDateTime to capture any difference in server and database times (skew or timezone).
+        LocalDateTime serverTime = LocalDateTime.now();
+        LocalDateTime databaseTime = new SqlSelector(scope, "SELECT CURRENT_TIMESTAMP").getObject(LocalDateTime.class);
+
+        return new ServerDatabaseTimeDifference(serverTime, databaseTime);
+    }
+
+    public record ServerDatabaseTimeDifference(LocalDateTime serverTime, LocalDateTime databaseTime)
+    {
+        public long getSeconds()
+        {
+            return Math.abs(Duration.between(serverTime, databaseTime).toSeconds());
+        }
+
+        public boolean exceedsWarningThreshold()
+        {
+            return getSeconds() > TIME_DIFFERENCE_WARNING_SECONDS;
+        }
+    }
+
+    // GH Issue #1223: Add a site configuration warning for administrators if the server and database clocks differ.
+    protected void addTimeDifferenceWarning(Warnings warnings, boolean showAllWarnings)
+    {
+        try
+        {
+            ServerDatabaseTimeDifference difference = getServerDatabaseTimeDifference(DbScope.getLabKeyScope());
+
+            if (difference.exceedsWarningThreshold())
+                warnings.add(getTimeDifferenceWarning(difference.getSeconds()));
+            else if (showAllWarnings)
+                warnings.add(getTimeDifferenceWarning(TIME_DIFFERENCE_WARNING_SECONDS + 1));
+        }
+        catch (Exception e)
+        {
+            LOG.warn("Unable to compare web server and database server times", e);
+        }
+    }
+
+    private HtmlString getTimeDifferenceWarning(long seconds)
+    {
+        return HtmlString.of("The web server and database server times differ by " + seconds + " seconds. " +
+            "LabKey Server often relies on comparing timestamps stored in the database with timestamps generated by " +
+            "the web server, so this difference can lead to data integrity issues. Synchronize the clocks on these servers.");
+    }
+
     public abstract List<SQLFragment> getChangeStatements(TableChange change);
 
     public abstract void purgeTempSchema(Map<String, TempTableTracker> createdTableNames);
@@ -2017,7 +2070,7 @@ protected void trackTempTables(Map<String, TempTableTracker> createdTableNames)
     // Defragment an index, if necessary
     public void defragmentIndex(DbSchema schema, String tableSelectName, String indexName)
     {
-        // By default do nothing
+        // By default, do nothing
     }
 
     public boolean isTableExists(DbScope scope, String schema, String name)
@@ -2045,7 +2098,7 @@ public boolean hasTriggers(DbSchema scope, String schema, String tableName)
 
     /**
      *
-     * @return true If the dialect is one supported for the backend LabKey database. ie, Postgres or SQL Server
+     * @return true If the dialect is one supported for the backend LabKey database. i.e., Postgres or SQL Server
      */
     public boolean isLabKeyDbDialect()
     {
@@ -2419,5 +2472,37 @@ public void testProcedureIdentifierQuoting()
             // A dotted name is rejected: schema and procedure are separate parameters, so a '.' in a single component is an ambiguous schema-qualified reference
             assertThrows(IllegalArgumentException.class, () -> dialect.buildProcedureCall(core.getName(), "a.b", 0, false, false, scope));
         }
+
+        // GH Issue #1223: Verify the server/database clock-skew arithmetic and warning threshold
+        @Test
+        public void testServerDatabaseTimeDifference()
+        {
+            LocalDateTime base = LocalDateTime.of(2026, 7, 6, 12, 0, 0);
+
+            // Identical times: zero difference, no warning
+            ServerDatabaseTimeDifference equal = new ServerDatabaseTimeDifference(base, base);
+            assertEquals(0, equal.getSeconds());
+            assertFalse(equal.exceedsWarningThreshold());
+
+            // Comfortably below the threshold: no warning
+            ServerDatabaseTimeDifference below = new ServerDatabaseTimeDifference(base, base.plusSeconds(5));
+            assertEquals(5, below.getSeconds());
+            assertFalse(below.exceedsWarningThreshold());
+
+            // Exactly at the threshold: no warning (comparison is strictly greater-than)
+            ServerDatabaseTimeDifference atThreshold = new ServerDatabaseTimeDifference(base, base.plusSeconds(TIME_DIFFERENCE_WARNING_SECONDS));
+            assertEquals(TIME_DIFFERENCE_WARNING_SECONDS, atThreshold.getSeconds());
+            assertFalse(atThreshold.exceedsWarningThreshold());
+
+            // Just past the threshold: warning
+            ServerDatabaseTimeDifference over = new ServerDatabaseTimeDifference(base, base.plusSeconds(TIME_DIFFERENCE_WARNING_SECONDS + 1));
+            assertEquals(TIME_DIFFERENCE_WARNING_SECONDS + 1, over.getSeconds());
+            assertTrue(over.exceedsWarningThreshold());
+
+            // Difference is absolute: database clock behind the web server is treated the same as ahead
+            ServerDatabaseTimeDifference behind = new ServerDatabaseTimeDifference(base, base.minusSeconds(TIME_DIFFERENCE_WARNING_SECONDS + 1));
+            assertEquals(TIME_DIFFERENCE_WARNING_SECONDS + 1, behind.getSeconds());
+            assertTrue(behind.exceedsWarningThreshold());
+        }
     }
 }

core/src/org/labkey/core/admin/admin.jsp

@@ -19,7 +19,7 @@
 <%@ page import="org.apache.commons.lang3.StringUtils" %>
 <%@ page import="org.labkey.api.admin.AdminBean" %>
 <%@ page import="org.labkey.api.data.DbScope" %>
-<%@ page import="org.labkey.api.data.SqlSelector" %>
+<%@ page import="org.labkey.api.data.dialect.SqlDialect" %>
 <%@ page import="org.labkey.api.files.FileContentService" %>
 <%@ page import="org.labkey.api.module.DefaultModule" %>
 <%@ page import="org.labkey.api.module.Module" %>
@@ -31,18 +31,18 @@
 <%@ page import="org.labkey.api.settings.AppProps" %>
 <%@ page import="org.labkey.api.util.Formats" %>
 <%@ page import="org.labkey.api.util.HtmlString"%>
+<%@ page import="org.labkey.api.util.HtmlStringBuilder" %>
 <%@ page import="org.labkey.api.view.NavTree" %>
 <%@ page import="org.labkey.core.admin.AdminController" %>
 <%@ page import="java.text.DecimalFormat" %>
-<%@ page import="java.time.Duration" %>
-<%@ page import="java.time.LocalDateTime" %>
 <%@ page import="java.time.format.DateTimeFormatter" %>
 <%@ page import="java.util.ArrayList" %>
 <%@ page import="java.util.Collection" %>
 <%@ page import="java.util.Comparator" %>
 <%@ page import="java.util.Map" %>
 <%@ page import="java.util.TreeMap" %>
 <%@ page import="org.apache.commons.lang3.Strings" %>
+<%@ page import="org.labkey.api.util.DateUtil" %>
 <%@ page extends="org.labkey.api.jsp.JspBase" %>
 <%@ taglib prefix="labkey" uri="http://www.labkey.org/taglib" %>
 <%
@@ -55,6 +55,7 @@
     .lk-admin-section { display: none; }
     .header-title { margin-bottom: 5px; }
     .header-link { margin: 0 5px 20px 0; }
+    .lk-server-time-warning { color: red; }
 </style>
 <div class="row">
     <div class="col-sm-12 col-md-3">
@@ -91,16 +92,19 @@
             <br/>
 <%
     row = 0;
+    DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern(DateUtil.getJsonDateTimeFormatString());
+    String timeCellCls = "";
+    HtmlString warning = HtmlString.EMPTY_STRING;
 
-    LocalDateTime serverTime = LocalDateTime.now();
-    LocalDateTime databaseTime = new SqlSelector(DbScope.getLabKeyScope(), "SELECT CURRENT_TIMESTAMP").getObject(LocalDateTime.class);
-    long duration = Math.abs(Duration.between(serverTime, databaseTime).toSeconds());
-
-    // Warn if greater than this many seconds
-    long warningSeconds = 10;
-
-    HtmlString style = unsafe(duration > warningSeconds ? " style=\"color:red;\"" : "");
-    HtmlString warning = unsafe(duration > warningSeconds ? " - Warning: Web and database server times differ by " + duration + " seconds!" : "");
+    SqlDialect.ServerDatabaseTimeDifference timeDifference = SqlDialect.getServerDatabaseTimeDifference(DbScope.getLabKeyScope());
+    if (timeDifference.exceedsWarningThreshold())
+    {
+        timeCellCls = "lk-server-time-warning";
+        warning = HtmlStringBuilder.of(" - Warning: Web and database server times differ by ")
+                .append(timeDifference.getSeconds())
+                .append(" seconds!")
+                .getHtmlString();
+    }
 %>
             <h4>Runtime Information</h4>
             <table class="labkey-data-region-legacy labkey-show-borders">
@@ -125,9 +129,9 @@
                 <tr class="<%=getShadeRowClass(row++)%>"><td>Working Dir</td><td><%=h(AdminBean.workingDir)%></td></tr>
                 <tr class="<%=getShadeRowClass(row++)%>"><td>Server GUID</td><td style="font-family:monospace"><%=h(AdminBean.serverGuid)%></td></tr>
                 <tr class="<%=getShadeRowClass(row++)%>"><td>Server Session GUID</td><td style="font-family:monospace"><%=h(AdminBean.serverSessionGuid)%></td></tr>
-                <tr class="<%=getShadeRowClass(row++)%>"><td>Server Startup Time</td><td<%=style%>><%=h(AdminBean.serverStartupTime)%></td></tr>
-                <tr class="<%=getShadeRowClass(row++)%>"><td>Web Server Time</td><td<%=style%>><%=h(serverTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")))%><%=warning%></td></tr>
-                <tr class="<%=getShadeRowClass(row++)%>"><td>Database Server Time</td><td<%=style%>><%=h(databaseTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")))%><%=warning%></td></tr>
+                <tr class="<%=getShadeRowClass(row++)%>"><td>Server Startup Time</td><td><%=h(AdminBean.serverStartupTime)%></td></tr>
+                <tr class="<%=getShadeRowClass(row++)%>"><td>Web Server Time</td><td class="<%=h(timeCellCls)%>"><%=h(dateTimeFormatter.format(timeDifference.serverTime()))%><%=warning%></td></tr>
+                <tr class="<%=getShadeRowClass(row++)%>"><td>Database Server Time</td><td class="<%=h(timeCellCls)%>"><%=h(dateTimeFormatter.format(timeDifference.databaseTime()))%><%=warning%></td></tr>
             </table>
         </labkey:panel>
         <labkey:panel id="links" className="lk-admin-section">

core/src/org/labkey/core/dialect/PostgreSql92Dialect.java

@@ -371,6 +371,8 @@ public void addAdminWarningMessages(Warnings warnings, boolean showAllWarnings)
         super.addAdminWarningMessages(warnings, showAllWarnings);
         if (showAllWarnings)
             warnings.add(HtmlString.of(PostgreSqlDialectFactory.getStandardWarningMessage("has not been tested against", getMajorVersion() + ".x")));
+
+        addTimeDifferenceWarning(warnings, showAllWarnings);
     }
 
     private int getIdentifierMaxByteLength()

experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java

@@ -139,8 +139,6 @@
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.sql.Timestamp;
-import java.time.Duration;
-import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -1406,19 +1404,15 @@ private _MaterializedQueryHelper getOrCreateMQH()
      */
     private static boolean isIncrementalUpdateDisabled()
     {
+        // Disable if web server and database time differ
         if (_incrementalUpdateDisabled == null)
         {
-            // borrowed from core/admin.jsp
-            LocalDateTime databaseTime = new SqlSelector(DbScope.getLabKeyScope(), "SELECT CURRENT_TIMESTAMP").getObject(LocalDateTime.class);
-            LocalDateTime serverTime = LocalDateTime.now();
-
-            // Disable if greater than this many seconds
-            long thresholdSeconds = 10;
-            long deltaSeconds = Math.abs(Duration.between(serverTime, databaseTime).toSeconds());
-            _incrementalUpdateDisabled = deltaSeconds > thresholdSeconds;
+            DbScope scope = DbScope.getLabKeyScope();
+            SqlDialect.ServerDatabaseTimeDifference difference = SqlDialect.getServerDatabaseTimeDifference(scope);
+            _incrementalUpdateDisabled = difference.exceedsWarningThreshold();
 
             if (_incrementalUpdateDisabled)
-                _log.warn("Incremental update disabled for samples. Web and database server time differ by {} seconds which exceeds the threshold of {} seconds. You may experience degraded sample query performance.", deltaSeconds, thresholdSeconds);
+                _log.warn("Incremental update disabled for samples. Web and database server time differ by {} seconds which exceeds the threshold of {} seconds. You may experience degraded sample query performance.", difference.getSeconds(), SqlDialect.TIME_DIFFERENCE_WARNING_SECONDS);
         }
 
         return _incrementalUpdateDisabled;