diff --git a/api/src/org/labkey/api/data/dialect/SqlDialect.java b/api/src/org/labkey/api/data/dialect/SqlDialect.java index 946edfa95b2..90eb7cc62c0 100644 --- a/api/src/org/labkey/api/data/dialect/SqlDialect.java +++ b/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; @@ -79,7 +80,10 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; +import java.sql.Timestamp; import java.sql.Types; +import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Calendar; @@ -95,6 +99,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; @@ -863,25 +868,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 +1995,61 @@ public final Collection 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 Instants, not wall-clock values, so the skew is measured correctly even when the servers are in different time zones. + Instant serverTime = Instant.now(); + Instant databaseTime = new SqlSelector(scope, "SELECT CURRENT_TIMESTAMP").getObject(Timestamp.class).toInstant(); + + return new ServerDatabaseTimeDifference(serverTime, databaseTime); + } + + public record ServerDatabaseTimeDifference(Instant serverTime, Instant 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 getChangeStatements(TableChange change); public abstract void purgeTempSchema(Map createdTableNames); @@ -2017,7 +2071,7 @@ protected void trackTempTables(Map 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) @@ -2419,5 +2473,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() + { + Instant base = Instant.parse("2026-07-06T12:00:00Z"); + + // 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()); + } } } diff --git a/core/src/org/labkey/core/admin/admin.jsp b/core/src/org/labkey/core/admin/admin.jsp index befbdb1837e..0bd0d93f3a1 100644 --- a/core/src/org/labkey/core/admin/admin.jsp +++ b/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,11 +31,11 @@ <%@ 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.ZoneId" %> <%@ page import="java.time.format.DateTimeFormatter" %> <%@ page import="java.util.ArrayList" %> <%@ page import="java.util.Collection" %> @@ -43,6 +43,7 @@ <%@ 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 +56,7 @@ .lk-admin-section { display: none; } .header-title { margin-bottom: 5px; } .header-link { margin: 0 5px 20px 0; } + .lk-server-time-warning { color: red; }
@@ -91,16 +93,19 @@
<% row = 0; + DateTimeFormatter dateTimeFormatter = DateTimeFormatter.ofPattern(DateUtil.getJsonDateTimeFormatString()).withZone(ZoneId.systemDefault()); + 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(); + } %>

Runtime Information

@@ -125,9 +130,9 @@ - ><%=h(AdminBean.serverStartupTime)%> - ><%=h(serverTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")))%><%=warning%> - ><%=h(databaseTime.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")))%><%=warning%> + + +
Working Dir<%=h(AdminBean.workingDir)%>
Server GUID<%=h(AdminBean.serverGuid)%>
Server Session GUID<%=h(AdminBean.serverSessionGuid)%>
Server Startup Time
Web Server Time
Database Server Time
Server Startup Time<%=h(AdminBean.serverStartupTime)%>
Web Server Time<%=h(dateTimeFormatter.format(timeDifference.serverTime()))%><%=warning%>
Database Server Time<%=h(dateTimeFormatter.format(timeDifference.databaseTime()))%><%=warning%>
diff --git a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java b/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java index 55fa2623127..6c09ab7fbc0 100644 --- a/core/src/org/labkey/core/dialect/PostgreSql92Dialect.java +++ b/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() diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 63bb3b8e304..38f71e66222 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/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;