Bugfix NGT flush logic #2598
Bugfix NGT flush logic #2598
Conversation
Signed-off-by: hlts2 <[email protected]>
WalkthroughWalkthroughThe changes involve modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant NGT as NGT Instance
participant Directory as Directory System
NGT->>Directory: Check n.inMem state
alt n.inMem is true
Directory-->>NGT: Do not delete directory
else n.inMem is false
Directory-->>NGT: Delete directory
end
sequenceDiagram
participant NGT as NGT Instance
NGT->>NGT: Check state for Len
alt State is valid
NGT-->>Caller: Return length
else State is invalid
NGT-->>Caller: Return 0
end
NGT->>NGT: Check state for InsertVQueueBufferLen
alt State is valid
NGT-->>Caller: Return buffer length
else State is invalid
NGT-->>Caller: Return 0
end
NGT->>NGT: Check state for DeleteVQueueBufferLen
alt State is valid
NGT-->>Caller: Return buffer length
else State is invalid
NGT-->>Caller: Return 0
end
sequenceDiagram
participant Server as gRPC Server
Server->>Server: Call CreateIndex
alt Flushing in progress
Server-->>Client: Return error codes.Aborted
else No flushing
Server-->>Client: Proceed with index creation
end
Server->>Server: Call CreateAndSaveIndex
alt Flushing in progress
Server-->>Client: Return error codes.Aborted
else No flushing
Server-->>Client: Proceed with index creation and saving
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[CHATOPS:HELP] ChatOps commands.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2598 +/- ##
==========================================
- Coverage 24.53% 24.37% -0.17%
==========================================
Files 530 531 +1
Lines 45522 45867 +345
==========================================
+ Hits 11171 11182 +11
- Misses 33602 33934 +332
- Partials 749 751 +2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: hlts2 <[email protected]>
Deploying vald with
|
| Latest commit: |
c293365
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://25714553.vald.pages.dev |
| Branch Preview URL: | https://fix-ngt-index.vald.pages.dev |
Signed-off-by: hlts2 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/agent/core/ngt/service/ngt.go (2 hunks)
Additional context used
GitHub Check: codecov/patch
pkg/agent/core/ngt/service/ngt.go
[warning] 1252-1252: pkg/agent/core/ngt/service/ngt.go#L1252
Added line #L1252 was not covered by tests
[warning] 1947-1947: pkg/agent/core/ngt/service/ngt.go#L1947
Added line #L1947 was not covered by tests
[warning] 1954-1954: pkg/agent/core/ngt/service/ngt.go#L1954
Added line #L1954 was not covered by tests
[warning] 1961-1961: pkg/agent/core/ngt/service/ngt.go#L1961
Added line #L1961 was not covered by tests
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/agent/core/ngt/service/ngt.go (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/agent/core/ngt/service/ngt.go
Signed-off-by: hlts2 <[email protected]>
| case n.IsFlushing(): | ||
| return errors.ErrFlushingIsInProgress | ||
| } | ||
|
|
||
| ic := n.vq.IVQLen() + n.vq.DVQLen() |
There was a problem hiding this comment.
Note
Added logic to check if a vqueue reference could be nil if a flush is running.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/agent/core/ngt/service/ngt.go (4 hunks)
Files skipped from review due to trivial changes (1)
- pkg/agent/core/ngt/service/ngt.go
Signed-off-by: hlts2 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/agent/core/ngt/service/ngt.go (5 hunks)
Additional comments not posted (2)
pkg/agent/core/ngt/service/ngt.go (2)
1302-1309: Verify the handling of delete and insert operations inCreateIndex.The function
CreateIndexhandles index creation by processing delete and insert operations from the vector queue. It is crucial to verify that these operations maintain data integrity and that the conditions under which they are processed are correctly implemented.Verification successful
Verification Successful: Handling of Delete and Insert Operations in
CreateIndexThe
CreateIndexfunction's handling of delete and insert operations is adequately tested across various test files in the codebase. These tests ensure that the function maintains data integrity and correctly processes operations under different conditions.
- The function is referenced in multiple test files, indicating comprehensive testing coverage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the handling of delete and insert operations in `CreateIndex`. # Test: Search for the function usage and check the handling of operations. rg --type go -A 10 $'CreateIndex'Length of output: 256432
Line range hint
1252-1281: Verify the handling of new instance creation inRegenerateIndexes.The function
RegenerateIndexescorrectly handles the regeneration of indexes by locking and setting appropriate flags. However, the transfer of settings and configurations to the new instance should be explicitly verified to ensure that all parameters are correctly applied.
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
Signed-off-by: hlts2 <[email protected]>
|
[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/fix%2Fngt%2Findex/f6b38052fe4de74fd4aba4521b348e2fcdfe0363 |
Signed-off-by: hlts2 <[email protected]>
* fix: bugfix flush logic Signed-off-by: hlts2 <[email protected]> * fix: nil check for flushing Signed-off-by: hlts2 <[email protected]> * fix: add flush check logic Signed-off-by: hlts2 <[email protected]> * fix: nil check bug Signed-off-by: hlts2 <[email protected]> * fix: add nil check Signed-off-by: hlts2 <[email protected]> * fix: return err when the flush process is executing Signed-off-by: hlts2 <[email protected]> * fix: add error check for flushing Signed-off-by: hlts2 <[email protected]> * fix: error message Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vqueue initialization Signed-off-by: hlts2 <[email protected]> * fix: disable commentout Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: nil set to kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: copy ngt service object for flushing Signed-off-by: hlts2 <[email protected]> * fix: deleted unnecessary nil check Signed-off-by: hlts2 <[email protected]> * fix: variable name Signed-off-by: hlts2 <[email protected]> --------- Signed-off-by: hlts2 <[email protected]> Co-authored-by: Yusuke Kato <[email protected]>
* fix: bugfix flush logic * fix: nil check for flushing * fix: add flush check logic * fix: nil check bug * fix: add nil check * fix: return err when the flush process is executing * fix: add error check for flushing * fix: error message * fix: disable kvs and vqueue initialization * fix: disable commentout * fix: disable kvs and vq * fix: nil set to kvs and vq * fix: copy ngt service object for flushing * fix: deleted unnecessary nil check * fix: variable name --------- Signed-off-by: hlts2 <[email protected]> Co-authored-by: Hiroto Funakoshi <[email protected]> Co-authored-by: Yusuke Kato <[email protected]> Co-authored-by: Kiichiro YUKAWA <[email protected]>
* fix: bugfix flush logic Signed-off-by: hlts2 <[email protected]> * fix: nil check for flushing Signed-off-by: hlts2 <[email protected]> * fix: add flush check logic Signed-off-by: hlts2 <[email protected]> * fix: nil check bug Signed-off-by: hlts2 <[email protected]> * fix: add nil check Signed-off-by: hlts2 <[email protected]> * fix: return err when the flush process is executing Signed-off-by: hlts2 <[email protected]> * fix: add error check for flushing Signed-off-by: hlts2 <[email protected]> * fix: error message Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vqueue initialization Signed-off-by: hlts2 <[email protected]> * fix: disable commentout Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: nil set to kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: copy ngt service object for flushing Signed-off-by: hlts2 <[email protected]> * fix: deleted unnecessary nil check Signed-off-by: hlts2 <[email protected]> * fix: variable name Signed-off-by: hlts2 <[email protected]> --------- Signed-off-by: hlts2 <[email protected]> Co-authored-by: Yusuke Kato <[email protected]>
* fix: bugfix flush logic Signed-off-by: hlts2 <[email protected]> * fix: nil check for flushing Signed-off-by: hlts2 <[email protected]> * fix: add flush check logic Signed-off-by: hlts2 <[email protected]> * fix: nil check bug Signed-off-by: hlts2 <[email protected]> * fix: add nil check Signed-off-by: hlts2 <[email protected]> * fix: return err when the flush process is executing Signed-off-by: hlts2 <[email protected]> * fix: add error check for flushing Signed-off-by: hlts2 <[email protected]> * fix: error message Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vqueue initialization Signed-off-by: hlts2 <[email protected]> * fix: disable commentout Signed-off-by: hlts2 <[email protected]> * fix: disable kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: nil set to kvs and vq Signed-off-by: hlts2 <[email protected]> * fix: copy ngt service object for flushing Signed-off-by: hlts2 <[email protected]> * fix: deleted unnecessary nil check Signed-off-by: hlts2 <[email protected]> * fix: variable name Signed-off-by: hlts2 <[email protected]> --------- Signed-off-by: hlts2 <[email protected]> Co-authored-by: Yusuke Kato <[email protected]>
Description
If a CRUD operation such as insert is executed again after flush, a nil pointer error occurred.
Initialize with the RegenerateIndexes function, but do not rewrite the receiver. So I changed that the field is override.
Related Issue
Versions
Checklist
Special notes for your reviewer
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
inMemflag is false.New Features
ngtinstances, enhancing state duplication.