Skip to content

Feat/336/wal-recover #35200

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
xiao-77 wants to merge 17 commits into
base: 3.3.6
Choose a base branch
from
Open

Feat/336/wal-recover #35200

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
50b5164
feat(wal): add walRecoveryPolicy config parameter
xiao-77 Apr 22, 2026
4eeeb7b
refactor(wal): add replica parameter to walCheckAndRepairMeta
xiao-77 Apr 22, 2026
8be4d82
feat(wal): implement recovery policy decision logic
xiao-77 Apr 22, 2026
1f23a8a
feat(wal): pass replica count from vnode to WAL layer
xiao-77 Apr 22, 2026
8d8b4f5
feat(wal): add sync notification for WAL truncation
xiao-77 Apr 22, 2026
dbf6f82
feat(wal): remove sync notification (not needed)
xiao-77 Apr 22, 2026
281ce7b
refactor(wal): remove tsWalDeleteOnCorruption and implement proper WA…
xiao-77 Apr 22, 2026
224b376
Merge branch 'feat/336/wal-recover' of https://github.com/taosdata/TD…
xiao-77 Apr 22, 2026
f77e404
refactor(wal): encapsulate WAL truncation logic into dedicated function
xiao-77 Apr 22, 2026
4fb8017
feat(wal): truncate corrupted file at last valid entry instead of del…
xiao-77 Apr 23, 2026
5567cc6
test(wal): add comprehensive WAL recovery policy test cases
xiao-77 Apr 23, 2026
0967b0b
test(wal): add unit tests for WAL recovery policy
xiao-77 Apr 23, 2026
93d55a5
fix(test): fix WAL create database syntax, wal 1 -> wal_level 1
xiao-77 Apr 23, 2026
c98c398
refactor(test): split WAL recovery tests into separate files per test…
xiao-77 Apr 23, 2026
de6a4de
fix(wal): remove duplicate truncation in walTruncateCorruptedFiles an…
xiao-77 Apr 28, 2026
e970734
fix: ci problems.
xiao-77 Apr 28, 2026
7fb598d
feat: remove unused test.
xiao-77 Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/common/tglobal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ extern int64_t tsmaDataDeleteMark;
extern int64_t tsWalFsyncDataSizeLimit;
extern bool tsWalForceRepair;
extern bool tsWalDeleteOnCorruption;
// WAL recovery policy (only affects single replica)
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

Header comment says walRecoveryPolicy "only affects single replica", but the new logic checks it for all replica != 3 cases (so it would also affect replica=2 if that configuration exists). Please align the comment (and possibly the config semantics) with actual behavior to avoid misconfiguration.

Suggested change
// WAL recovery policy (only affects single replica)
// WAL recovery policy (applies when replica count is not 3, including single-replica deployments)

Copilot uses AI. Check for mistakes.
// 0 = refuse to start (default), 1 = delete corrupted and start
extern int32_t tsWalRecoveryPolicy;

// internal
extern bool tsDiskIDCheckEnabled;
Expand Down
2 changes: 2 additions & 0 deletions include/libs/sync/sync.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ const char* syncStr(ESyncState state);

int32_t syncNodeGetConfig(int64_t rid, SSyncCfg* cfg);

int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer);

Comment on lines +306 to +307
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

syncNotifyWalTruncated is declared in the public sync header, but there is no implementation in the repo (no definition found under source/libs/sync/src). Any new call site will fail to link, and the interface addition is incomplete as-is. Add the corresponding implementation (and ensure it’s exported from the sync library) or remove the declaration until it’s implemented.

Suggested change
int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer);

Copilot uses AI. Check for mistakes.
// util
int32_t syncSnapInfoDataRealloc(SSnapshot* pSnap, int32_t size);

Expand Down
2 changes: 1 addition & 1 deletion include/libs/wal/wal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ int32_t walInit(stopDnodeFn stopDnode);
void walCleanUp();

// handle open and ctl
SWal *walOpen(const char *path, SWalCfg *pCfg);
SWal *walOpen(const char *path, SWalCfg *pCfg, int32_t replica);
Comment thread
xiao-77 marked this conversation as resolved.
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

walOpen signature now requires a replica argument, but there are still call sites in the repo (e.g. source/libs/sync/test/*.cpp) using the old 2-arg form. This will break compilation; please update all remaining callers (or provide a backward-compatible wrapper/default) so the tree builds cleanly.

Copilot uses AI. Check for mistakes.
int32_t walAlter(SWal *, SWalCfg *pCfg);
int32_t walPersist(SWal *);
void walClose(SWal *);
Expand Down
5 changes: 5 additions & 0 deletions source/common/src/tglobal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,7 @@ bool tsStartUdfd = true;
int64_t tsWalFsyncDataSizeLimit = (100 * 1024 * 1024L);
bool tsWalForceRepair = 0;
bool tsWalDeleteOnCorruption = false;
int32_t tsWalRecoveryPolicy = 0; // Default: refuse to start for single replica

// ttl
bool tsTtlChangeOnWrite = false; // if true, ttl delete time changes on last write
Expand Down Expand Up @@ -982,6 +983,7 @@ static int32_t taosAddServerCfg(SConfig *pCfg) {
TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "syncRoutineReportInterval", tsRoutineReportInterval, 5, 600, CFG_SCOPE_SERVER, CFG_DYN_SERVER,CFG_CATEGORY_LOCAL));
TAOS_CHECK_RETURN(cfgAddBool(pCfg, "syncLogHeartbeat", tsSyncLogHeartbeat, CFG_SCOPE_SERVER, CFG_DYN_SERVER,CFG_CATEGORY_LOCAL));
TAOS_CHECK_RETURN(cfgAddBool(pCfg, "walDeleteOnCorruption", tsWalDeleteOnCorruption, CFG_SCOPE_SERVER, CFG_DYN_NONE,CFG_CATEGORY_LOCAL));
TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "walRecoveryPolicy", tsWalRecoveryPolicy, 0, 1, CFG_SCOPE_SERVER, CFG_DYN_NONE, CFG_CATEGORY_LOCAL));
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

Config key walDeleteOnCorruption is removed and replaced with walRecoveryPolicy, which is a breaking change for existing deployments/config files and docs. If backward compatibility is required, consider continuing to accept walDeleteOnCorruption as an alias (or at least emit a clear warning) and update documentation accordingly.

Suggested change
TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "walRecoveryPolicy", tsWalRecoveryPolicy, 0, 1, CFG_SCOPE_SERVER, CFG_DYN_NONE, CFG_CATEGORY_LOCAL));
TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "walRecoveryPolicy", tsWalRecoveryPolicy, 0, 1, CFG_SCOPE_SERVER, CFG_DYN_NONE, CFG_CATEGORY_LOCAL));
TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "walDeleteOnCorruption", tsWalRecoveryPolicy, 0, 1, CFG_SCOPE_SERVER, CFG_DYN_NONE, CFG_CATEGORY_LOCAL));

Copilot uses AI. Check for mistakes.

TAOS_CHECK_RETURN(cfgAddInt32(pCfg, "syncTimeout", tsSyncTimeout, 0, 60 * 24 * 2 * 1000, CFG_SCOPE_SERVER, CFG_DYN_SERVER,CFG_CATEGORY_GLOBAL));

Expand Down Expand Up @@ -2146,6 +2148,9 @@ static int32_t taosSetServerCfg(SConfig *pCfg) {
TAOS_CHECK_GET_CFG_ITEM(pCfg, pItem, "walDeleteOnCorruption");
tsWalDeleteOnCorruption = pItem->bval;

TAOS_CHECK_GET_CFG_ITEM(pCfg, pItem, "walRecoveryPolicy");
tsWalRecoveryPolicy = pItem->i32;

TAOS_RETURN(TSDB_CODE_SUCCESS);
}

Expand Down
2 changes: 1 addition & 1 deletion source/dnode/mnode/impl/src/mndMain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ static int32_t mndInitWal(SMnode *pMnode) {
}
#endif

pMnode->pWal = walOpen(path, &cfg);
pMnode->pWal = walOpen(path, &cfg, pMnode->syncMgmt.numOfReplicas);
if (pMnode->pWal == NULL) {
code = TSDB_CODE_MND_RETURN_VALUE_NULL;
if (terrno != 0) code = terrno;
Expand Down
2 changes: 1 addition & 1 deletion source/dnode/vnode/src/vnd/vnodeOpen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ SVnode *vnodeOpen(const char *path, int32_t diskPrimary, STfs *pTfs, SMsgCb msgC
TAOS_UNUSED(ret);

vInfo("vgId:%d, start to open vnode wal", TD_VID(pVnode));
pVnode->pWal = walOpen(tdir, &(pVnode->config.walCfg));
pVnode->pWal = walOpen(tdir, &(pVnode->config.walCfg), pVnode->config.syncCfg.replicaNum);
if (pVnode->pWal == NULL) {
vError("vgId:%d, failed to open vnode wal since %s. wal:%s", TD_VID(pVnode), tstrerror(terrno), tdir);
goto _err;
Expand Down
2 changes: 2 additions & 0 deletions source/libs/sync/inc/syncInt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ int32_t syncNodeDynamicQuorum(const SSyncNode* pSyncNode);
bool syncNodeIsMnode(SSyncNode* pSyncNode);
int32_t syncNodePeerStateInit(SSyncNode* pSyncNode);

int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer);

#ifdef __cplusplus
}
#endif
Expand Down
5 changes: 5 additions & 0 deletions source/libs/sync/src/syncMain.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4090,3 +4090,8 @@ bool syncNodeCanChange(SSyncNode* pSyncNode) {
return true;
}
#endif

int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer) {
sInfo("vgId:%d, notified sync module: WAL truncated to ver:%" PRId64, vgId, truncatedVer);
return TSDB_CODE_SUCCESS;
}
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

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

medium

The implementation of syncNotifyWalTruncated currently only logs a message. To fulfill the PR's objective of 'Enabling Raft to detect and sync missing logs', this function should update the state of the corresponding sync node (e.g., resetting its applied index or marking it as needing a snapshot/sync).

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

syncNotifyWalTruncated() is added to public headers, but there are no call sites in the repo. If the intent is to notify Raft after WAL truncation/recovery, wire this into the WAL recovery path (e.g., after walTruncateCorruptedFiles succeeds) and pass the actual truncated version; otherwise keep the symbol internal or remove it to avoid exposing an unused API surface.

Suggested change
int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer) {
sInfo("vgId:%d, notified sync module: WAL truncated to ver:%" PRId64, vgId, truncatedVer);
return TSDB_CODE_SUCCESS;
}

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

syncNotifyWalTruncated() is added as a public/internal API but is not called anywhere in this PR, and currently only logs a message without informing any SSyncNode state. This doesn’t match the PR description (“Call notification after three-replica recovery / Enable Raft to detect and sync missing logs”). Either wire this into the WAL truncation/recovery path (and have it trigger whatever raft re-sync behavior is required) or keep it internal until there’s an actual caller/behavior.

Copilot uses AI. Check for mistakes.
2 changes: 1 addition & 1 deletion source/libs/wal/inc/walInt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ int32_t walRemoveMeta(SWal* pWal);
int32_t walRollImpl(SWal* pWal);
int32_t walRollFileInfo(SWal* pWal);
int32_t walScanLogGetLastVer(SWal* pWal, int32_t fileIdx, int64_t* lastVer);
int32_t walCheckAndRepairMeta(SWal* pWal);
int32_t walCheckAndRepairMeta(SWal* pWal, int32_t replica);
int64_t walChangeWrite(SWal* pWal, int64_t ver);

int32_t walCheckAndRepairIdx(SWal* pWal);
Expand Down
39 changes: 37 additions & 2 deletions source/libs/wal/src/walMeta.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
#include "tutil.h"
#include "walInt.h"

extern int32_t syncNotifyWalTruncated(int32_t vgId, int64_t truncatedVer);
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

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

medium

Instead of using an extern declaration inside a source file, it is better practice to include the header file where the function is declared. This ensures type safety and a single source of truth for the function signature.

#include "sync.h"


bool FORCE_INLINE walLogExist(SWal* pWal, int64_t ver) {
return !walIsEmpty(pWal) && walGetFirstVer(pWal) <= ver && walGetLastVer(pWal) >= ver;
}
Expand Down Expand Up @@ -517,7 +519,7 @@ void walRegfree(regex_t* ptr) {
regfree(ptr);
}

int32_t walCheckAndRepairMeta(SWal* pWal) {
int32_t walCheckAndRepairMeta(SWal* pWal, int32_t replica) {
// load log files, get first/snapshot/last version info
int32_t code = 0;
int32_t lino = 0;
Expand All @@ -526,6 +528,8 @@ int32_t walCheckAndRepairMeta(SWal* pWal) {
regex_t logRegPattern, idxRegPattern;
TdDirPtr pDir = NULL;
SArray* actualLog = NULL;
bool walTruncated = false;
int64_t truncatedVer = -1;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

walTruncated and truncatedVer are declared but never used, which is dead code and can trigger -Wunused-variable warnings in some build configurations. Either remove them or use them for the intended truncation notification/metadata tracking (e.g., record truncatedVer and propagate it to the sync module).

Suggested change
bool walTruncated = false;
int64_t truncatedVer = -1;

Copilot uses AI. Check for mistakes.

wInfo("vgId:%d, begin to repair meta, wal path:%s, first index:%" PRId64 ", last index:%" PRId64
", snapshot index:%" PRId64,
Expand Down Expand Up @@ -607,7 +611,31 @@ int32_t walCheckAndRepairMeta(SWal* pWal) {
if (lastVer < 0) {
if (code != TSDB_CODE_WAL_LOG_NOT_EXIST) {
wError("vgId:%d, failed to scan wal last index since %s", pWal->cfg.vgId, tstrerror(code));
if (tsWalDeleteOnCorruption) {

bool shouldRecover = false;
if (replica == 3) {
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

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

medium

The check for replica == 3 is too specific. TDengine supports other replica counts (e.g., 2 or 5). It is better to check if the node is part of a multi-replica group.

        if (replica > 1) {

shouldRecover = true;
walTruncated = true;
truncatedVer = pFileInfo->firstVer;
wInfo("vgId:%d, WAL corrupted at ver:%" PRId64 ", auto-recovery enabled for replica=3",
pWal->cfg.vgId, pFileInfo->firstVer);
} else {
shouldRecover = (tsWalRecoveryPolicy == 1);
if (shouldRecover) {
wWarn("vgId:%d, WAL corrupted at ver:%" PRId64 ", force recovery enabled by walRecoveryPolicy=1",
pWal->cfg.vgId, pFileInfo->firstVer);
} else {
wError("vgId:%d, WAL corrupted at ver:%" PRId64 ", refusing to start to prevent data loss",
pWal->cfg.vgId, pFileInfo->firstVer);
wError("vgId:%d, corrupted WAL files are preserved for manual inspection", pWal->cfg.vgId);
wError("vgId:%d, to force recovery with data loss, set 'walRecoveryPolicy 1' in taos.cfg and restart",
pWal->cfg.vgId);
code = TSDB_CODE_WAL_FILE_CORRUPTED;
goto _exit;
}
}

if (shouldRecover && tsWalDeleteOnCorruption) {
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The new "auto-recovery enabled for replica=3" path is inconsistent: when replica==3 and WAL scan fails, the code only recovers if tsWalDeleteOnCorruption is enabled; otherwise it falls through to _exit with an error code, causing walOpen() to fail despite logging that auto-recovery is enabled. Either perform the recovery action unconditionally for replica==3 (still preserving the corrupted dir), or make the log/error handling reflect the actual required config and set a deterministic return code.

Suggested change
if (shouldRecover && tsWalDeleteOnCorruption) {
if (replica == 3 || (shouldRecover && tsWalDeleteOnCorruption)) {

Copilot uses AI. Check for mistakes.
TAOS_RETURN(walRenameCorruptedDir(pWal));
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

When recovery triggers TAOS_RETURN(walRenameCorruptedDir(pWal)), the function returns before reaching the later syncNotifyWalTruncated() block, so the sync module is never notified in the main recovery path. If notification is required for replica=3 recovery, move the notification before the early return (or avoid returning early).

Suggested change
TAOS_RETURN(walRenameCorruptedDir(pWal));
code = walRenameCorruptedDir(pWal);

Copilot uses AI. Check for mistakes.
}
goto _exit;
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

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

high

There are two issues here:

  1. The notification to the sync module (intended at line 680) is unreachable because this block either returns via TAOS_RETURN or jumps to _exit before reaching the end of the function.
  2. For multi-replica setups, the log at line 620 claims auto-recovery is enabled, but the actual recovery is still gated by tsWalDeleteOnCorruption (which defaults to false). If recovery is intended to be automatic for clusters, this guard should be adjusted.
        if (shouldRecover && (tsWalDeleteOnCorruption || replica > 1)) {
          if (replica > 1) {
            syncNotifyWalTruncated(pWal->cfg.vgId, truncatedVer);
          }
          TAOS_RETURN(walRenameCorruptedDir(pWal));
        }
        goto _exit;

Expand Down Expand Up @@ -649,6 +677,13 @@ int32_t walCheckAndRepairMeta(SWal* pWal) {

TAOS_CHECK_EXIT(walLogEntriesComplete(pWal));

if (walTruncated && replica == 3) {
int32_t syncCode = syncNotifyWalTruncated(pWal->cfg.vgId, truncatedVer);
if (syncCode != TSDB_CODE_SUCCESS) {
wWarn("vgId:%d, failed to notify sync module, code:0x%x", pWal->cfg.vgId, syncCode);
}
}
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 22, 2026

Choose a reason for hiding this comment

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

high

This block is unreachable when walTruncated is true. As noted in the previous comment, any path that detects corruption and sets walTruncated = true results in an early exit from the function. The notification logic should be moved into the recovery block where the actual truncation/renaming occurs.

Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

walCheckAndRepairMeta() now calls syncNotifyWalTruncated(), but the wal static library does not link against sync (and sync already links against wal), so this introduces a circular/undefined-symbol risk (e.g. walTest links wal without sync). Replace this direct call with a decoupled mechanism such as a callback function pointer registered by sync at runtime, or a weak/no-op default implementation compiled into wal.

Copilot uses AI. Check for mistakes.

Comment thread
xiao-77 marked this conversation as resolved.
wInfo("vgId:%d, success to repair meta, wal path:%s, first index:%" PRId64 ", last index:%" PRId64
", snapshot index:%" PRId64,
pWal->cfg.vgId, pWal->path, pWal->vers.firstVer, pWal->vers.lastVer, pWal->vers.snapshotVer);
Expand Down
4 changes: 2 additions & 2 deletions source/libs/wal/src/walMgmt.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ int32_t walInitWriteFileForSkip(SWal *pWal) {
TAOS_RETURN(code);
}

SWal *walOpen(const char *path, SWalCfg *pCfg) {
SWal *walOpen(const char *path, SWalCfg *pCfg, int32_t replica) {
int32_t code = 0;
SWal *pWal = taosMemoryCalloc(1, sizeof(SWal));
if (pWal == NULL) {
Expand Down Expand Up @@ -206,7 +206,7 @@ SWal *walOpen(const char *path, SWalCfg *pCfg) {
wWarn("vgId:%d, failed to load meta, code:0x%x", pWal->cfg.vgId, code);
}
if (pWal->cfg.level != TAOS_WAL_SKIP) {
code = walCheckAndRepairMeta(pWal);
code = walCheckAndRepairMeta(pWal, replica);
if (code < 0) {
wError("vgId:%d, cannot open wal since repair meta file failed since %s", pWal->cfg.vgId, tstrerror(code));
goto _err;
Expand Down
Loading