Feature/wal log by destrex271 · Pull Request #52 · destrex271/LokiKV

destrex271 · 2025-09-29T03:55:25Z

Adds a basic wal
Adds a Control File
Adds the option to append to wal
Adds the option to flush wal to disk and start writing again

Pending:

Control File Initialization
Replay over latest checkpoint
inspect wal
timeline switchover

deepsource-io · 2025-09-29T03:55:50Z

Here's the code health analysis summary for commits 7bb0775..e82f3fe. View details on DeepSource ↗.

Analysis Summary

Analyzer	Status	Summary	Link
Rust	❌ Failure	❗ 50 occurences introduced 🎯 20 occurences resolved	View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

…nt pending

destrex271 · 2025-11-12T14:31:43Z

@copilot review this PR

Copilot

Pull Request Overview

This PR adds Write-Ahead Logging (WAL) functionality to LokiKV, including a control file system for managing WAL timelines and checkpoint metadata.

Introduces WALManager for recording and replaying database operations to WAL files
Adds ControlFile for persisting configuration and tracking checkpoint/timeline state
Integrates WAL recording into the put operations and checkpoint workflow
Adds DISPLAY_WAL command for inspecting WAL contents

Reviewed Changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
src/db/loki_kv/wal.rs	New WAL implementation with record management, file I/O, and replay functionality
src/db/loki_kv/control.rs	New control file implementation for managing WAL timeline and checkpoint metadata
src/db/loki_kv/control.toml	Configuration file defining WAL and checkpoint directory paths
src/db/loki_kv/loki_kv.rs	Integrates WAL manager into database operations and checkpoint process
src/db/loki_kv/persist.rs	Updates persistor to use control file for directory paths
src/db/loki_kv/mod.rs	Exports new control and wal modules
src/db/parser/parser.rs	Adds DISPLAY_WAL command enum and parsing
src/db/parser/lokiql.pest	Adds DISPLAY_WAL to grammar and allows trailing separators
src/db/parser/executor.rs	Implements DISPLAY_WAL command execution
src/db/server_multithread/server.rs	Changes checkpoint to use write lock instead of read lock, removes unused method
Cargo.toml	Adds toml dependency for control file serialization
Cargo.lock	Updates dependencies including serde and toml crates

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

Copilot · 2025-11-12T14:41:37Z

src/db/loki_kv/loki_kv.rs

-        for col in self.collections_bmap.iter(){
-            let pth = format!("{}/{}", get_checkpoint_directory(), get_current_timestamp()); 
+        for col in self.collections_bmap.iter() {
+            let pth = format!("{}/{}", get_checkpoint_directory(), checkpoint_id);


Unused variable: The pth variable is created but never used. This appears to be leftover from refactoring when the code was changed to pass get_control_file_path() to Persistor::new() instead of the path. Consider removing these lines.

Copilot · 2025-11-12T14:41:38Z

src/db/loki_kv/wal.rs

+            timestamp: timestamp,
+            collection_name: collection_name,
+            key: key,
+            value: value,


[nitpick] Redundant field initialization: When the field name matches the variable name, you can use shorthand syntax (e.g., WALRecord { timestamp, collection_name, key, value }).

Suggested change

timestamp: timestamp,

collection_name: collection_name,

key: key,

value: value,

timestamp,

collection_name,

key,

value,

Copilot · 2025-11-12T14:41:38Z

src/db/loki_kv/wal.rs

+    pub fn new_without_toml() -> Self {
+        let control_file = ControlFile::write(
+            "/home/akshat/lokikv/control.toml".to_string(),
+            0 as u64,
+            0 as u64,
+            "/home/akshat/lokikv/checkpoints".to_string(),
+            "/home/akshat/lokikv/wal".to_string(),
+        )
+        .unwrap();
+        let timeline = control_file.get_next_timeline_id();
+        WALManager {
+            control_file,
+            wal_records: Vec::new(),
+            cur_timeline: timeline,
+        }
+    }


Hardcoded absolute path: This function contains hardcoded paths specific to /home/akshat/lokikv/. These should be configurable via environment variables or the control file path parameter, similar to how the new() method works.

@copilot is it better to just remove the new_without_toml function? I don't see it being used as such

Copilot · 2025-11-12T14:41:38Z

src/db/loki_kv/wal.rs

+            0 as u64,
+            0 as u64,


[nitpick] Redundant type cast: 0 as u64 is unnecessary. Simply use 0 since the type is already specified in the function parameter.

Suggested change

0 as u64,

0 as u64,

0,

0,

Copilot · 2025-11-12T14:41:39Z

src/db/loki_kv/control.toml

+checkpoint_directory_path = "/home/akshat/lokikv/checkpoints"
+wal_directory_path = "/home/akshat/lokikv/wal"


Hardcoded absolute paths in configuration file: This TOML file contains hardcoded paths specific to /home/akshat/lokikv/. This configuration file should either use relative paths or be generated dynamically, and should not be committed to version control with user-specific absolute paths.

Suggested change

checkpoint_directory_path = "/home/akshat/lokikv/checkpoints"

wal_directory_path = "/home/akshat/lokikv/wal"

checkpoint_directory_path = "./checkpoints"

wal_directory_path = "./wal"

Copilot · 2025-11-12T14:41:41Z

src/db/loki_kv/loki_kv.rs

+        let checkpoint_id = get_current_timestamp_as_u64();
+
+        for col in self.collections_bmap_cust.iter() {
+            let pth = format!("{}/{}", get_checkpoint_directory(), checkpoint_id);


Unused variable: The pth variable is created but never used. This appears to be leftover from refactoring when the code was changed to pass get_control_file_path() to Persistor::new() instead of the path. Consider removing these lines.

Copilot · 2025-11-12T14:41:42Z

src/db/loki_kv/loki_kv.rs

+        let res = self.wal_manager.display_wal();
+        res


[nitpick] Redundant return statement: The explicit return is unnecessary here. Simply use res without the return keyword, which is more idiomatic in Rust.

Copilot · 2025-11-12T14:41:42Z

src/db/loki_kv/loki_kv.rs

        let mut collections_hmap: HashMap<String, Collection> = HashMap::new();
        let mut collections_bmap: HashMap<String, CollectionBTree> = HashMap::new();
        let mut collections_bmap_cust: HashMap<String, CollectionBTreeCustom> = HashMap::new();
+        let control_file_path = "/home/akshat/lokikv/control.toml";


Hardcoded absolute path: The control file path is hardcoded to /home/akshat/lokikv/control.toml. This path is user-specific and will fail on other systems. Use the get_control_file_path() function that's already defined in this file to make it configurable.

Suggested change

let control_file_path = "/home/akshat/lokikv/control.toml";

let control_file_path = get_control_file_path();

Copilot · 2025-11-12T14:41:42Z

src/db/loki_kv/loki_kv.rs


 use clap::builder::StringValueParser;
 use serde::{Deserialize, Serialize};
+use tokio::fs::File;


Unused import: tokio::fs::File is imported but never used in this file. Consider removing it.

Suggested change

use tokio::fs::File;

src/db/loki_kv/wal.rs

destrex271 added 12 commits May 15, 2025 12:21

work till now

6050897

buggy code: DO NOT RUN

52c3cd4

wal

555119e

removed some errors

a2bbc13

wal manager

d30ec35

adding control file

afc8803

Merge branch 'main' into feature/wal_log

0b85e29

added control file

ba8032e

base wal

016948d

Merge branch 'main' into feature/wal_log

8cee795

added basic utils to replay and add wal records

1cbd178

wal append and flush added

d1550d0

destrex271 added 2 commits October 5, 2025 09:43

display wal added

3c17bbd

checkpoint uses ctrl dir; wal display added; wal replay over checkpoi…

45198d7

…nt pending

destrex271 closed this Nov 12, 2025

destrex271 reopened this Nov 12, 2025

destrex271 requested a review from Copilot November 12, 2025 14:33

Copilot started reviewing on behalf of destrex271 November 12, 2025 14:33 View session

Copilot finished reviewing on behalf of destrex271 November 12, 2025 14:38

Copilot AI reviewed Nov 12, 2025

View reviewed changes

fix: update replay_wal to use deserialize_from

e82f3fe

destrex271 merged commit b6478c0 into main Nov 12, 2025
0 of 2 checks passed

-as u64,
-as u64,
+,
+,

		checkpoint_directory_path = "/home/akshat/lokikv/checkpoints"
		wal_directory_path = "/home/akshat/lokikv/wal"

	let control_file_path = "/home/akshat/lokikv/control.toml";
	let control_file_path = get_control_file_path();

Conversation

destrex271 commented Sep 29, 2025

Uh oh!

deepsource-io bot commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Analysis Summary

Uh oh!

destrex271 commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

destrex271 Nov 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

deepsource-io bot commented Sep 29, 2025 •

edited

Loading

destrex271 Nov 12, 2025 •

edited

Loading