Skip to content
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

[#76] wal 구현 #80

Closed
wants to merge 6 commits into from
Closed

[#76] wal 구현 #80

wants to merge 6 commits into from

Conversation

wHoIsDReAmer
Copy link
Collaborator

@wHoIsDReAmer wHoIsDReAmer commented Mar 20, 2024

resolved: #76

설명

간단하게 해당 PR이 어떤 내용인지 작성합니다. 이슈를 통해서 충분히 설명이 가능 하다고 생각하면 resolved만 작성하시면 됩
니다.

@wHoIsDReAmer wHoIsDReAmer linked an issue Mar 20, 2024 that may be closed by this pull request
@wHoIsDReAmer wHoIsDReAmer self-assigned this Mar 20, 2024
@myyrakle myyrakle changed the title 76 feature wal 구현 [#76] wal 구현 Mar 20, 2024
@myyrakle myyrakle self-requested a review March 20, 2024 16:33

let value = (header, Vec::new());

/// TODO
Copy link
Owner

Choose a reason for hiding this comment

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

todo 처리를 하려면 todo! 매크로를 쓰는 것도 좋은 방법

src/wal/wal.rs Outdated
let mut wal = Wal::default();

let mut wal_metadata = get_target_basepath();
wal_metadata.push("current_wal_index");
Copy link
Owner

Choose a reason for hiding this comment

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

이런 텍스트는 어디다 상수로 빼죠

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

추후에 상단에 const로 정의해놓겠습니다.

@@ -29,14 +32,22 @@ impl Server {

let (request_sender, mut request_receiver) = mpsc::channel::<ChannelRequest>(1000);

// shared WAL
// wal struct을 생성, Arc로 관리
let shared_wal = Arc::new(Wal::new().await);
Copy link
Owner

Choose a reason for hiding this comment

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

  1. wal을 글로벌로 하나만 두는게 의도한 건가요? 추후 wal을 테이블이나 블럭 단위로 분리할 계획인가요?
  2. 그리고 저걸로 뭔가 mutation을 하려면 Mutex로 바꿔야할 것 같은데 맞나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. 의도한게 맞습니다. 레고드에 테이블 정보랑 데이터베이스 정보가 다 들어있습니다.
  2. Mutex로 바꾸는게 맞는 것 같습니다.

src/wal/wal.rs Outdated
use super::{format::{BinaryFormatterImpl, LogFileHeader, MAGIC_NUMBER, VERSION}, record::LogRecord};

#[derive(Clone, Debug, Default)]
pub struct Wal {
Copy link
Owner

Choose a reason for hiding this comment

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

  1. 식별자명은 Wal이 나을까요? WAL이 나을까요? 이건 선택의 영역일 것 같네요.
  2. 타입명이 더 명확하면 좋지 않을까 싶군요. WalManager인지 뭔지 명확한 역할이 타입명에 드러나지 않는 것 같아서

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. WalManager가 괜찮을 것 같습니다. 다음 커밋 때 적용시키겠습니다.

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.

[feature] WAL 구현
2 participants