Skip to content

[2628-a2] feat(checkpoint): add createTime to memory checkpoint key#2637

Open
Takuka0311 wants to merge 3 commits into
alibaba:mainfrom
Takuka0311:feat/2631-checkpoint-createtime-key
Open

[2628-a2] feat(checkpoint): add createTime to memory checkpoint key#2637
Takuka0311 wants to merge 3 commits into
alibaba:mainfrom
Takuka0311:feat/2631-checkpoint-createtime-key

Conversation

@Takuka0311

Copy link
Copy Markdown
Collaborator

背景 / Motivation

Epic #2629 · Discussion #2628W1(checkpoint key 增加物理文件区分维度)内存层步骤

内存 checkpoint 的 key 为 (dev, inode, configName)无法区分复用同一 inode 的不同物理文件(同名重建时同 inode)→ 互相覆盖 → 恢复时丢失一方状态、从头重采。本 PR 在内存层给 key 增加 createTime 维度,根治同 inode 覆盖。

改动 / Changes

  • CheckPoint 增加 int64_t mCreateTime(默认 0),构造函数新增尾部默认形参 createTime = 0
  • CheckPointKey 增加 mCreateTimeoperator< 比较顺序改为 dev → inode → createTime → configName
  • AddCheckPointCheckPoint::mCreateTime 取值构建 key;GetCheckPoint / DeleteCheckPoint 新增 createTime 形参(默认 0)。
  • 兼容性:新增形参均默认 0,createTime == 0 时行为与旧逻辑完全一致,现有调用点(LogFileReaderEventDispatcherCheckpointManagerV2无需改动即可编译

范围锁 / Scope

仅改动(符合 Issue 范围锁):

  • core/file_server/checkpoint/CheckPointManager.h
  • core/file_server/checkpoint/CheckPointManager.cpp(仅内存 Add/Get/Delete 与 key/结构)
  • core/unittest/checkpoint/CheckpointManagerUnittest.cpp(新增 UT)

未涉及:磁盘 dump/load 的 create_time 序列化(A3)、createTime 实际填充(A4)、LogFileReader.* / EventDispatcher.* / CheckpointManagerV2.* / FileSystemUtil.*

Test plan

新增 UT(CheckpointManagerUnittest),本地实际跑通(Docker build image 增量编译 + 运行):

用例 断言
TestAddCheckPointSameInodeDifferentCreateTime 同 config、同 (dev,inode)、不同 createTime 两条 → map size==2,互不覆盖
TestSameNameRebuildNoOverwrite 同名 + 同 inode + 不同 createTime(重建场景)→ 旧/新 offset 各自保留
TestGetDeleteCheckPointWithCreateTime Get/Delete 按 createTime 精确命中
TestCreateTimeZeroBackwardCompatible 12 参构造 createTime 默认 0;默认形参 Get/Delete 与旧逻辑一致;同 inode 不同 config 仍分开

运行结果:

[==========] 5 tests from 1 test case ran. (3 ms total)
[  PASSED  ] 5 tests.

(含原有 TestSearchFilePathByDevInodeInDirectory,4 个为本 PR 新增)

构建/测试命令:以 loongcollector-build-linux:2.1.17 镜像编译 checkpoint_manager_unittest(devtoolset-9 + ASAN,-Werror 通过),并直接运行该 UT 二进制。clang-format 18 已格式化。

Closes #2631

The in-memory checkpoint key was (dev, inode, configName), which cannot
distinguish different physical files that reuse the same inode (e.g. a
same-name rebuild). Such checkpoints overwrote each other, so on recovery
one file's state was lost and it was re-collected from the beginning.

Add an int64_t createTime dimension to CheckPointKey and CheckPoint so
these entries no longer collide. Comparison order is
dev -> inode -> createTime -> configName. createTime defaults to 0, which
preserves the legacy keying behavior; disk dump/load serialization and the
actual createTime plumbing are handled in follow-up steps (A3/A4).

Closes alibaba#2631
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Takuka0311

Copy link
Copy Markdown
Collaborator Author

这个pr的改动是不是没有调用点,纯粹为后续的阶段搭建框架?

@Takuka0311

Copy link
Copy Markdown
Collaborator Author

你的观察是准确的:本 PR 在生产代码路径上确实没有会填入非零 createTime 的调用点,是 Epic #2629 的分阶段搭建。 已逐一核对全部调用点:

生产调用点(全部走默认 createTime=0,行为与改动前完全一致)

  • LogFileReader.cpp:288new CheckPoint(...) 未传 createTime → 0
  • LogFileReader.cpp:304 / 336 / 376AddCheckPoint / GetCheckPoint / DeleteCheckPoint 未传 createTime → 0
  • CheckpointManagerV2.cpp:235GetCheckPoint(...) 未传 createTime → 0
  • CheckPointManager.cpp LoadFileCheckPoint(281/316)— 磁盘反序列化尚未读取 create_time → 0

为什么此 PR 不含真正的填值调用点
本 issue #2631 [A2] 的范围锁明确禁止改 LogFileReader.* / EventDispatcher.* / CheckpointManagerV2.* / 磁盘 dump/load 序列化 —— 而这些正是未来真正填充 createTime 的位置。它们分属 Epic 的其它阶段:

安全性 / 兼容性

  • mCreateTime 与三个方法的新参数均默认 00 语义为“未设置”,key 排序退化为原 (dev, inode, configName),因此生产行为零变化
  • 已有 UT 覆盖:TestCreateTimeZeroBackwardCompatible(默认 0 与旧逻辑一致)、TestAddCheckPointSameInodeDifferentCreateTimeTestSameNameRebuildNoOverwrite(同名重建不再覆盖)、TestGetDeleteCheckPointWithCreateTime(精确命中)。
  • CI 在当前 commit 68e74a4f 全绿(含 BuildCoreUT)。

综上,这是有意为之的增量:本 PR 只引入 key 维度与结构并以 UT 证明机制正确,wiring 按范围锁留给 A3 及后续阶段。若你更希望把 A2 与后续 wiring 合并为单一 PR、或调整阶段边界,请示下,我另行开范围处理(当前不在本 issue 允许改动集内)。

from=agent action=none

@Takuka0311 Takuka0311 marked this pull request as ready for review July 6, 2026 02:48
@yyuuttaaoo yyuuttaaoo requested a review from Copilot July 6, 2026 03:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR extends the in-memory checkpoint key to include createTime, preventing collisions when different physical files reuse the same (dev, inode) and ensuring checkpoints don’t overwrite each other in same-inode rebuild scenarios.

Changes:

  • Add mCreateTime to CheckPoint and CheckPointKey, and update CheckPointKey::operator< to include createTime in ordering.
  • Update in-memory Add/Get/Delete to use the extended key with a default createTime = 0 for backward compatibility.
  • Add unit tests covering same-inode/different-createTime storage and backward-compatible behavior.

Reviewed changes

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

File Description
core/file_server/checkpoint/CheckPointManager.h Adds mCreateTime, extends key structure and public APIs with defaulted createTime.
core/file_server/checkpoint/CheckPointManager.cpp Uses createTime when building the map key for Add/Get/Delete in memory.
core/unittest/checkpoint/CheckpointManagerUnittest.cpp Adds UTs validating non-overwrite behavior and default-0 compatibility.

Comment thread core/file_server/checkpoint/CheckPointManager.h Outdated
Comment thread core/file_server/checkpoint/CheckPointManager.h Outdated
Comment thread core/file_server/checkpoint/CheckPointManager.cpp Outdated
Comment thread core/unittest/checkpoint/CheckpointManagerUnittest.cpp Outdated
Address PR review: introduce kUnsetCreateTime so the "createTime not
assigned" value is explicit rather than a bare 0, document that the
memory key is exact-match (not a wildcard), and use insert_or_assign in
AddCheckPoint to replace the same key in a single lookup. Tests now hold
owning unique_ptr and release() into AddCheckPoint.

Refs alibaba#2631
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Takuka0311 Takuka0311 changed the title feat(checkpoint): [A2] add createTime to memory checkpoint key [2628-a2] feat(checkpoint): add createTime to memory checkpoint key Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[2628-a2] checkpoint 内存 key 增加 createTime 维度(根治同 inode 覆盖)

2 participants