-
Notifications
You must be signed in to change notification settings - Fork 180
feat: add index state tracking for crash recovery #3471
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.
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
base: master
Are you sure you want to change the base?
Conversation
Added index state management to track whether indexing operations completed cleanly or were interrupted. This enables intelligent recovery on service restart by avoiding unnecessary global updates when the index was properly shut down. 1. Added IndexState enum with Clean, Dirty, and Unknown states 2. Implemented state persistence in index status file with JSON serialization 3. TaskManager now marks state as Dirty when starting tasks and Clean when all tasks complete successfully 4. TextIndexDBus checks state during silent startup to skip global updates for clean shutdowns 5. Added cleanup logic to mark state as Dirty if tasks are interrupted during service shutdown 6. Refactored status file handling to use shared helper functions Log: Improved indexing service reliability with crash recovery mechanism Influence: 1. Test service restart after clean shutdown - should skip global update 2. Test service restart after interrupted indexing - should trigger global update 3. Verify state persistence across service restarts 4. Test index creation when no database exists (should proceed normally) 5. Verify task queue management with multiple indexing operations 6. Test service shutdown during active indexing operations feat: 添加索引状态跟踪以支持崩溃恢复 新增索引状态管理功能,用于跟踪索引操作是否正常完成或被中断。这使服务重启 时能够智能恢复,避免在索引正常关闭时执行不必要的全局更新。 1. 添加包含 Clean、Dirty 和 Unknown 状态的 IndexState 枚举 2. 在索引状态文件中实现状态持久化,使用 JSON 序列化 3. TaskManager 在启动任务时将状态标记为 Dirty,所有任务成功完成后标记 为 Clean 4. TextIndexDBus 在静默启动时检查状态,对于干净关闭跳过全局更新 5. 添加清理逻辑,在服务关闭时如果任务被中断则将状态标记为 Dirty 6. 重构状态文件处理以使用共享辅助函数 Log: 通过崩溃恢复机制提升索引服务可靠性 Influence: 1. 测试服务在干净关闭后重启 - 应跳过全局更新 2. 测试服务在索引中断后重启 - 应触发全局更新 3. 验证状态在服务重启间的持久性 4. 测试不存在数据库时的索引创建(应正常进行) 5. 验证多索引操作的任务队列管理 6. 测试在活跃索引操作期间的服务关闭
Reviewer's GuideIntroduces an IndexState crash-recovery mechanism backed by the existing JSON status file, wires it into the TaskManager lifecycle and TextIndexDBus startup/shutdown paths, and refactors status-file reads/writes through shared helpers so the index can skip unnecessary global updates after a clean shutdown while forcing updates after interrupted work. Sequence diagram for silent startup index recovery behaviorsequenceDiagram
actor Service
participant TextIndexDBus
participant Private
participant TaskManager
participant IndexUtility
Service->>TextIndexDBus: handleSlientStart()
TextIndexDBus->>Private: execute_silent_start_lambda()
Private->>TextIndexDBus: IndexDatabaseExists()
alt database does not exist
Private->>TaskManager: startTask(Create, pathsToProcess, true)
TaskManager->>IndexUtility: setIndexState(Dirty)
else database exists
Private->>IndexUtility: getIndexState()
alt state is Clean
Private-->>Service: skip_global_update()
else state Dirty or Unknown
Private->>TaskManager: startTask(Update, pathsToProcess, true)
TaskManager->>IndexUtility: setIndexState(Dirty)
end
end
Sequence diagram for cleanup marking dirty state on unfinished worksequenceDiagram
actor Service
participant TextIndexDBus
participant Private
participant TaskManager
participant IndexUtility
Service->>TextIndexDBus: cleanup()
TextIndexDBus->>Private: fsEventController.setEnabledNow(false)
TextIndexDBus->>TaskManager: hasRunningTask()
TaskManager-->>TextIndexDBus: runningFlag
TextIndexDBus->>TaskManager: hasQueuedTasks()
TaskManager-->>TextIndexDBus: queuedFlag
alt unfinished work
TextIndexDBus->>IndexUtility: setIndexState(Dirty)
end
TextIndexDBus->>TextIndexDBus: StopCurrentTask()
State diagram for IndexState crash-recovery lifecyclestateDiagram-v2
[*] --> Unknown
Unknown --> Dirty: indexing_started
Clean --> Dirty: indexing_started
Dirty --> Clean: all_tasks_completed_successfully
Dirty --> Dirty: interrupted_shutdown
Unknown --> Clean: clean_status_written
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 1 issue, and left some high level feedback:
- Consider using QSaveFile (or a similar atomic write mechanism) in writeStatusJson instead of QFile::WriteOnly to avoid leaving a partially written/invalid status file if the process crashes during the write.
- getIndexState silently returns Unknown for missing/invalid state values; adding a debug or warning log in this path would make it easier to diagnose legacy/corrupted status files and understand why a global update is being triggered.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider using QSaveFile (or a similar atomic write mechanism) in writeStatusJson instead of QFile::WriteOnly to avoid leaving a partially written/invalid status file if the process crashes during the write.
- getIndexState silently returns Unknown for missing/invalid state values; adding a debug or warning log in this path would make it easier to diagnose legacy/corrupted status files and understand why a global update is being triggered.
## Individual Comments
### Comment 1
<location> `src/services/textindex/utils/indexutility.cpp:37-46` </location>
<code_context>
+}
+
+// Internal helper: Write status JSON object to file
+bool writeStatusJson(const QJsonObject &obj)
+{
+ QFile file(statusFilePath());
+ QDir().mkpath(QFileInfo(file).absolutePath());
+
+ if (!file.open(QIODevice::WriteOnly)) {
+ fmWarning() << "Failed to write index status to:" << file.fileName();
+ return false;
+ }
+
+ file.write(QJsonDocument(obj).toJson());
+ file.close();
+ return true;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider handling JSON serialization/parsing errors and logging them explicitly.
Right now malformed JSON is treated as an empty object in `readStatusJson()`, and `writeStatusJson()` assumes `QJsonDocument(obj).toJson()` always succeeds. If the status file is corrupt, we silently drop all fields and overwrite the file with no diagnostics. Please consider checking `QJsonParseError` in `fromJson` and logging invalid content so operators can distinguish "file missing" from "file corrupt" and investigate accordingly.
Suggested implementation:
```cpp
QJsonParseError parseError;
const QByteArray data = file.readAll();
QJsonDocument doc = QJsonDocument::fromJson(data, &parseError);
file.close();
if (parseError.error != QJsonParseError::NoError) {
fmWarning() << "Failed to parse index status JSON from:" << file.fileName()
<< "-" << parseError.errorString();
return QJsonObject();
}
if (!doc.isObject()) {
fmWarning() << "Index status JSON root is not an object in:" << file.fileName();
return QJsonObject();
}
return doc.object();
}
// Internal helper: Write status JSON object to file
bool writeStatusJson(const QJsonObject &obj)
{
QFile file(statusFilePath());
QDir().mkpath(QFileInfo(file).absolutePath());
if (!file.open(QIODevice::WriteOnly)) {
fmWarning() << "Failed to write index status to:" << file.fileName();
return false;
}
const QByteArray data = QJsonDocument(obj).toJson();
const qint64 bytesWritten = file.write(data);
file.close();
if (bytesWritten != data.size()) {
fmWarning() << "Failed to fully write index status to:" << file.fileName()
<< "- wrote" << bytesWritten << "of" << data.size() << "bytes";
return false;
}
return true;
}
```
If `QJsonParseError` is not already included in this compilation unit (directly or transitively), add `#include <QJsonParseError>` near the other Qt JSON includes at the top of `indexutility.cpp`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| bool writeStatusJson(const QJsonObject &obj) | ||
| { | ||
| QFile file(statusFilePath()); | ||
| QDir().mkpath(QFileInfo(file).absolutePath()); | ||
|
|
||
| if (!file.open(QIODevice::WriteOnly)) { | ||
| fmWarning() << "Failed to write index status to:" << file.fileName(); | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): Consider handling JSON serialization/parsing errors and logging them explicitly.
Right now malformed JSON is treated as an empty object in readStatusJson(), and writeStatusJson() assumes QJsonDocument(obj).toJson() always succeeds. If the status file is corrupt, we silently drop all fields and overwrite the file with no diagnostics. Please consider checking QJsonParseError in fromJson and logging invalid content so operators can distinguish "file missing" from "file corrupt" and investigate accordingly.
Suggested implementation:
QJsonParseError parseError;
const QByteArray data = file.readAll();
QJsonDocument doc = QJsonDocument::fromJson(data, &parseError);
file.close();
if (parseError.error != QJsonParseError::NoError) {
fmWarning() << "Failed to parse index status JSON from:" << file.fileName()
<< "-" << parseError.errorString();
return QJsonObject();
}
if (!doc.isObject()) {
fmWarning() << "Index status JSON root is not an object in:" << file.fileName();
return QJsonObject();
}
return doc.object();
}
// Internal helper: Write status JSON object to file
bool writeStatusJson(const QJsonObject &obj)
{
QFile file(statusFilePath());
QDir().mkpath(QFileInfo(file).absolutePath());
if (!file.open(QIODevice::WriteOnly)) {
fmWarning() << "Failed to write index status to:" << file.fileName();
return false;
}
const QByteArray data = QJsonDocument(obj).toJson();
const qint64 bytesWritten = file.write(data);
file.close();
if (bytesWritten != data.size()) {
fmWarning() << "Failed to fully write index status to:" << file.fileName()
<< "- wrote" << bytesWritten << "of" << data.size() << "bytes";
return false;
}
return true;
}
If QJsonParseError is not already included in this compilation unit (directly or transitively), add #include <QJsonParseError> near the other Qt JSON includes at the top of indexutility.cpp.
Added index state management to track whether indexing operations completed cleanly or were interrupted. This enables intelligent recovery on service restart by avoiding unnecessary global updates when the index was properly shut down.
Log: Improved indexing service reliability with crash recovery mechanism
Influence:
feat: 添加索引状态跟踪以支持崩溃恢复
新增索引状态管理功能,用于跟踪索引操作是否正常完成或被中断。这使服务重启
时能够智能恢复,避免在索引正常关闭时执行不必要的全局更新。
Log: 通过崩溃恢复机制提升索引服务可靠性
Influence:
Summary by Sourcery
Introduce persistent index state tracking to enable crash-aware recovery and avoid unnecessary global reindexing after clean shutdowns.
New Features:
Enhancements: