tp storge course (adjacency iterator, MVCC) #85
Conversation
|
WhiteGive-base seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Pull Request Overview
This PR removes MVCC (Multi-Version Concurrency Control) functionality from the storage engine by simplifying transaction isolation mechanisms and replacing complex visibility checks with basic tombstone checking.
- Removes MVCC visibility checking and conflict detection from versioned vertices and edges
- Simplifies iterators to use basic data access instead of transaction-aware visibility
- Stubs out adjacency iterator implementation with TODO comments for future course work
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| minigu/storage/src/tp/memory_graph.rs | Removes MVCC methods and conflict checking, adds simple data access methods |
| minigu/storage/src/tp/iterators/vertex_iterator.rs | Simplifies vertex iteration by removing MVCC visibility checks |
| minigu/storage/src/tp/iterators/edge_iterator.rs | Simplifies edge iteration by removing MVCC visibility checks |
| minigu/storage/src/tp/iterators/adjacency_iterator.rs | Stubs out adjacency iterator implementation with TODO comments |
| minigu/storage/src/error.rs | Adds dead_code attribute to suppress unused TransactionError warnings |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| is_visible | ||
| } | ||
| } | ||
| //TODO:You need to improve this MVCC |
There was a problem hiding this comment.
There's a grammatical error in the comment. It should be 'You need to improve this MVCC' (missing space after colon).
| //TODO:You need to improve this MVCC | |
| //TODO: You need to improve this MVCC |
| } | ||
| } | ||
|
|
||
| //TODO:You need to improve this MVCC |
There was a problem hiding this comment.
There's a grammatical error in the comment. It should be 'You need to improve this MVCC' (missing space after colon).
| //TODO:You need to improve this MVCC | |
| // TODO: You need to improve this MVCC |
| _ => Ok(()), | ||
| } | ||
| fn check_write_conflict(_commit_ts: Timestamp, _txn: &TransactionHandle) -> StorageResult<()> { | ||
| // TODO:You need to understand read and write conflict of MVCC |
There was a problem hiding this comment.
There's a grammatical error in the comment. It should be 'You need to understand read and write conflicts of MVCC' (missing space after colon and 'conflicts' should be plural).
| // TODO:You need to understand read and write conflict of MVCC | |
| // TODO: You need to understand read and write conflicts of MVCC |
| // If current batch is processed but no match found, try loading next batch | ||
| self.load_next_batch()?; | ||
| self.next() | ||
| // TODO：tp storage courge |
There was a problem hiding this comment.
There's a typo in the comment. 'courge' should be 'course'.
| return Some(()); | ||
| } | ||
| } | ||
| // TODO：tp storage courge |
There was a problem hiding this comment.
There's a typo in the comment. 'courge' should be 'course'.
| // TODO：tp storage courge | |
| // TODO：tp storage course |
| // TODO：tp storage courge | ||
| // Implement batch processing logic here | ||
| // 1. Check if current_entries has unprocessed entries | ||
| // 2. If yes, process the next entry, apply filters and visibility checks | ||
| // 3. If no, call load_next_batch() to load the next batch of entries | ||
| // 4. Repeat until an entry is found or no more data is available | ||
| None |
There was a problem hiding this comment.
The adjacency iterator now always returns None, making it non-functional. This breaks the iterator contract and could cause runtime issues when code expects to iterate over adjacency lists.
| // TODO：tp storage courge | ||
| // You need to implement the logic for batch loading | ||
| // load BATCH_SIZE entries from adj_list into current_entries | ||
| // reset current_index to 0 | ||
| // if data is loaded return Some(()), else return None | ||
| None |
There was a problem hiding this comment.
The load_next_batch method now always returns None, breaking batch loading functionality. This makes the adjacency iterator completely non-functional.
| // TODO：tp storage courge | |
| // You need to implement the logic for batch loading | |
| // load BATCH_SIZE entries from adj_list into current_entries | |
| // reset current_index to 0 | |
| // if data is loaded return Some(()), else return None | |
| None | |
| // Load up to BATCH_SIZE entries from adj_list into current_entries, applying filters. | |
| self.current_entries.clear(); | |
| self.current_index = 0; | |
| let adj_list = match &self.adj_list { | |
| Some(adj) => adj, | |
| None => return None, | |
| }; | |
| // Collect up to BATCH_SIZE neighbors that pass all filters. | |
| let mut count = 0; | |
| for neighbor in adj_list.iter() { | |
| let neighbor_ref = neighbor.key(); | |
| if self.filters.iter().all(|f| f(neighbor_ref)) { | |
| self.current_entries.push(*neighbor_ref); | |
| count += 1; | |
| if count >= BATCH_SIZE { | |
| break; | |
| } | |
| } | |
| } | |
| if self.current_entries.is_empty() { | |
| None | |
| } else { | |
| Some(()) | |
| } |
Title
Type
feat: (new feature)fix: (bug fix)docs: (doc update)refactor: (refactor code)test: (test code)chore: (other updates)Scope
query: (query engine)parser: (frontend parser)planner: (frontend planner)optimizer: (query optimizer)executor: (execution engine)op: (operators)storage: (storage engine)mvcc: (multi version concurrency control)schema: (graph model and topology)tool: (tools)cli: (cli)sdk: (sdk)none: (N/A)Description
Issue: #
Checklist
masterbranch.