-
Notifications
You must be signed in to change notification settings - Fork 86
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
Separate shadow pages from wal records and rework wal to use serializer #3204
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
#pragma once | ||
|
||
#include <typeinfo> | ||
|
||
#include "common/assert.h" | ||
|
||
namespace kuzu { | ||
|
@@ -8,11 +10,15 @@ namespace common { | |
template<typename FROM, typename TO> | ||
TO ku_dynamic_cast(FROM old) { | ||
#if defined(KUZU_RUNTIME_CHECKS) || !defined(NDEBUG) | ||
TO newVal = dynamic_cast<TO>(old); | ||
if constexpr (std::is_pointer<FROM>()) { | ||
KU_ASSERT(newVal != nullptr); | ||
try { | ||
TO newVal = dynamic_cast<TO>(old); | ||
if constexpr (std::is_pointer<FROM>()) { | ||
KU_ASSERT(newVal != nullptr); | ||
} | ||
return newVal; | ||
} catch (std::bad_cast& e) { | ||
KU_ASSERT(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the benefit of the assert here? It doesn't seem like it would be any clearer of an error than the exception. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just that it's easier to debug with ku_assert than catching the |
||
} | ||
return newVal; | ||
#else | ||
return reinterpret_cast<TO>(old); | ||
#endif | ||
|
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.
Is this always going to be correct? What if the last page in the file is only partially filled?
From where it's being used before it seems to me that it will continue reading the empty data at the end of the page, which will mess up the check for whether or not the last record is a WAL.
I think there should be some way of tracking how many records are in the WAL, or alternatively have a record type of
0
be used to mark invalid records, and then as long as the buffer gets reset to zero (which the bufferedfilewriter is doing) then it can break after it reaches a record with a type of0
.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.
bufferSize = std::min(fileSize - fileOffset, BUFFER_SIZE);
bufferSize
is bounded by the actual fileSize. so it won't read empty data at the end of the last page if it's partially filled.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.
Ensuring wal is not corrupted is another thing we need to handle. I feel we might need to do checksum for records.