refactor(timezone): standardize checkTime initialization to local timezone offset across dnode and mnode by Tony2h · Pull Request #35057 · taosdata/TDengine

Tony2h · 2026-04-03T07:34:27Z

… ms across dnode and mnode implementations

Description

Issue(s)

Close/close/Fix/fix/Resolve/resolve: Issue Link

Checklist

Please check the items in the checklist if applicable.

Is the user manual updated?
Are the test cases passed and automated?
Is there no significant decrease in test coverage?

… ms across dnode and mnode implementations

Copilot

Pull request overview

This PR refactors how checkTime is initialized in both mnode and dnode status/cluster-config paths, aiming to standardize the value (now set to 0) and updates inline documentation for the field.

Changes:

Replace taosParseTime("1970-01-01 00:00:00.00", ...) with direct checkTime = 0 initialization in mnode open logic and dnode status request population.
Add/replace struct field comments describing checkTime semantics in internal mnode state and the cluster config message struct.
Minor cleanup from removing parsing/error-handling blocks (but introduces a duplicate assignment site in dmSendStatusReq).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
source/dnode/mnode/impl/src/mndMain.c	Sets `pMnode->checkTime` directly to `0` during `mndOpen()` initialization.
source/dnode/mnode/impl/inc/mndInt.h	Adds a block comment documenting `SMnode::checkTime`.
source/dnode/mgmt/mgmt_dnode/src/dmHandle.c	Sets `req.clusterCfg.checkTime = 0` when building status request (now twice in the function).
include/common/tmsg.h	Updates `SClusterCfg::checkTime` comment to a block comment and removes the old “1970...” note.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T07:38:46Z

source/dnode/mnode/impl/src/mndMain.c

-    terrno = code;
-    return NULL;
-  }
+  pMnode->checkTime = 0;


Setting pMnode->checkTime to 0 makes the timezone consistency check in mndCheckClusterCfgPara ineffective: the check only errors when both timezone strings differ and checkTime differs. With both sides now using 0, a real timezone mismatch will not be detected. Consider keeping checkTime as a deterministic value derived from the node’s configured timezone (e.g., parse a fixed local datetime using the global/default timezone or store an explicit tz offset) or update the check logic accordingly.

Copilot · 2026-04-03T07:38:46Z

source/dnode/mgmt/mgmt_dnode/src/dmHandle.c

-      0) {
-    dError("failed to parse time since %s", tstrerror(code));
-  }
+  req.clusterCfg.checkTime = 0;


req.clusterCfg.checkTime is assigned twice in this function (earlier and again here). The second assignment is redundant and can mask future edits (e.g., if the first assignment is later changed to a computed value). Keep a single initialization for checkTime in one place.

Suggested change

req.clusterCfg.checkTime = 0;

Copilot · 2026-04-03T07:38:46Z

source/dnode/mgmt/mgmt_dnode/src/dmHandle.c

+  req.clusterCfg.checkTime = 0;
  memcpy(req.clusterCfg.timezone, tsTimezoneStr, TD_TIMEZONE_LEN);


Changing checkTime to a constant 0 disables the intended semantic comparison used by mnode when validating dnode timezone (it checks timezone string mismatch but allows equivalent timezones when the derived checkTime matches). If the goal is to avoid environment-dependent parsing, compute a stable value that still reflects the configured timezone (e.g., derived offset) or adjust the validation logic to continue catching genuine mismatches.

Copilot · 2026-04-03T07:38:47Z

include/common/tmsg.h

+    Unix epoch in ms (conventionally 0).
+    Dnode/mnode must agree; paired with timezone in cluster cfg check.


The new comment describes checkTime as “Unix epoch in ms (conventionally 0)”, but current usage in mndCheckClusterCfgPara treats checkTime as a derived value to help validate timezone equivalence. Please update this comment to reflect the actual purpose/meaning of checkTime (or, if semantics are intentionally changing to constant 0, update the validation logic and any other documentation accordingly).

Suggested change

Unix epoch in ms (conventionally 0).

Dnode/mnode must agree; paired with timezone in cluster cfg check.

Reference timestamp in ms used together with timezone during cluster

configuration checks to verify timezone equivalence across dnode/mnode.

This is a derived comparison value, not a field that is conventionally 0.

Copilot · 2026-04-03T07:38:47Z

source/dnode/mnode/impl/inc/mndInt.h

+    Unix epoch in ms (conventionally 0).
+    Dnode/mnode must agree; paired with timezone in cluster cfg check.


The comment implies checkTime is always the Unix epoch (0), but the field is used in cluster cfg validation to disambiguate timezone equivalence. Please align this comment with the field’s real semantics (or change the validation logic if checkTime is meant to be constant).

Suggested change

Unix epoch in ms (conventionally 0).

Dnode/mnode must agree; paired with timezone in cluster cfg check.

Reference timestamp in ms used together with timezone during

cluster cfg validation to disambiguate timezone equivalence.

Dnode/mnode must agree on this value.

gemini-code-assist

Code Review

This pull request simplifies the initialization of checkTime by replacing the parsing of a hardcoded date string with a direct assignment of 0, representing the Unix epoch. It also adds descriptive comments to the checkTime field in several header files to clarify its purpose and synchronization requirements. Feedback suggests using a named constant instead of the literal 0 to enhance code readability and maintainability.

source/dnode/mgmt/mgmt_dnode/src/dmHandle.c

-      0) {
-    dError("failed to parse time since %s", tstrerror(code));
-  }
+  req.clusterCfg.checkTime = 0;


source/dnode/mnode/impl/src/mndMain.c

-    terrno = code;
-    return NULL;
-  }
+  pMnode->checkTime = 0;


…econds

… dmSendStatusReq and mndOpen

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T02:43:19Z

source/dnode/mnode/impl/src/mndMain.c

+  pMnode->checkTime = (int64_t)taosGetLocalTimezoneOffset(&code);
+  if (code != 0) {
    taosMemoryFree(pMnode);
+    mError("failed to get local timezone offset, since %s", tstrerror(code));
    terrno = code;
    return NULL;


On failure, this path frees pMnode without destroying the rwlock initialized just above. The previous code called taosThreadRwlockDestroy(&pMnode->lock) on error; the new code should do the same before freeing to avoid leaking OS lock resources.

Copilot · 2026-04-07T02:43:19Z

include/common/tmsg.h

+  /*
+    Local timezone UTC offset in seconds (east-positive, e.g. +28800 for
+    Asia/Shanghai).  Derived from taosGetLocalTimezoneOffset() on each
+    status report.  Paired with the timezone string in
+    mndCheckClusterCfgPara: a mismatch is reported only when both the
+    timezone string AND this offset differ.
+  */
+  int64_t       checkTime;
  char          timezone[TD_TIMEZONE_LEN];  // tsTimezone


The PR title/description says checkTime is standardized to initialization "0", but the code now sets it to the local timezone UTC offset (seconds) via taosGetLocalTimezoneOffset(). Please update the PR title/description (or rename the field) so reviewers/users aren’t misled about the semantics change.

…val in mndOpen

refactor(time): standardize checkTime initialization to Unix epoch in…

08a89c9

… ms across dnode and mnode implementations

Tony2h requested review from dapan1121 and guanshengliang as code owners April 3, 2026 07:34

Copilot AI review requested due to automatic review settings April 3, 2026 07:34

taosdata-bot bot added the team engine label Apr 3, 2026

Copilot started reviewing on behalf of Tony2h April 3, 2026 07:35 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

gemini-code-assist bot reviewed Apr 3, 2026

View reviewed changes

Tony2h added 2 commits April 7, 2026 10:30

fix(timezone): update checkTime to use local timezone UTC offset in s…

fc11c87

…econds

fix(timezone): handle errors when retrieving local timezone offset in…

1d04f2e

… dmSendStatusReq and mndOpen

Copilot AI review requested due to automatic review settings April 7, 2026 02:40

Copilot started reviewing on behalf of Tony2h April 7, 2026 02:41 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

fix(timezone): improve error logging for local timezone offset retrie…

15984b9

…val in mndOpen

Tony2h changed the title ~~refactor(timezone): standardize checkTime initialization to 0 across dnode and mnode implementations~~ refactor(timezone): standardize checkTime initialization to local timezone offset across dnode and mnode Apr 7, 2026

		req.clusterCfg.checkTime = 0;
		memcpy(req.clusterCfg.timezone, tsTimezoneStr, TD_TIMEZONE_LEN);

		Unix epoch in ms (conventionally 0).
		Dnode/mnode must agree; paired with timezone in cluster cfg check.

-    Unix epoch in ms (conventionally 0).
-    Dnode/mnode must agree; paired with timezone in cluster cfg check.
+    Reference timestamp in ms used together with timezone during cluster
+    configuration checks to verify timezone equivalence across dnode/mnode.
+    This is a derived comparison value, not a field that is conventionally 0.

Conversation

Tony2h commented Apr 3, 2026

Description

Issue(s)

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

This comment was marked as duplicate.

Uh oh!

This comment was marked as duplicate.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Tony2h Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants