-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
test(506): regarded as a successful closure when session not found #609
Conversation
server/wal/codec/v1.go
Outdated
@@ -29,11 +30,7 @@ import ( | |||
// Size: Length of the payload data | |||
// Payload: Byte stream as long as specified by the payload size. | |||
|
|||
// Idx File: |
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.
This is removing a valid comment explaining the header format
common/client_pool.go
Outdated
"io" | ||
"log/slog" | ||
"strings" | ||
"sync" | ||
"time" | ||
|
||
"google.golang.org/grpc/keepalive" |
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.
We should move this refactoring (and all the similar) into a separate PR
oxia/options_list.go
Outdated
@@ -46,8 +46,7 @@ func (u *useIndex) applyRangeScan(opts *rangeScanOptions) { | |||
opts.secondaryIndexName = &u.indexName | |||
} | |||
|
|||
// UseIndex let the users specify a different index to follow for the |
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.
Why removing this part of the comment?
server/wal/codec/v2.go
Outdated
// +--------------+-----------+-----------+-----+ | ||
// | CRC(4Bytes) | Index(4B) | Index(4B) | ... | | ||
// +--------------+-----------+-----------+-----+ | ||
// CRC: 32bit hash computed over the payload using CRC. |
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.
Same, we shouldn't be removing this comment
@merlimat all other changes have been removed(it was auto changed by |
Fix: #506