Skip to content

Refactor transaction handling and update index management#435

Merged
JinHai-CN merged 4 commits intoinfiniflow:mainfrom
loloxwg:replay_index_new
Jan 10, 2024
Merged

Refactor transaction handling and update index management#435
JinHai-CN merged 4 commits intoinfiniflow:mainfrom
loloxwg:replay_index_new

Conversation

@loloxwg
Copy link
Copy Markdown
Member

@loloxwg loloxwg commented Jan 10, 2024

What problem does this PR solve?

This commit involves several updates to improve the handling of indexes and exceptions in database transactions. These updates range from enhancing transaction methods such as SaveIndexFile and PhysicalCreateIndexFinish for better error management, to improving storage efficiency through replacing WriteIndexFile function with WriteIndexToMemory. An enhancement to the transaction module for more accurate and efficient checking for existing keys has been also implemented.

Issue link:

What is changed and how it works?

Code changes

  • Has Code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (knn performance test)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Note for reviewer

This commit involves several updates to improve the handling of indexes and exceptions in database transactions. These updates range from enhancing transaction methods such as SaveIndexFile and PhysicalCreateIndexFinish for better error management, to improving storage efficiency through replacing WriteIndexFile function with WriteIndexToMemory. An enhancement to the transaction module for more accurate and efficient checking for existing keys has been also implemented.
@loloxwg loloxwg mentioned this pull request Jan 10, 2024
2 tasks
This change refactors `logical_planner.cpp` to instantiate `base_index_ptr` within the loop, thereby improving scope control and possibly reducing memory usage. Moreover, it aligns with the good programming practice of declaring variables in the smallest meaningful scope.
Comment thread src/storage/meta/entry/segment_entry.cpp
Comment thread src/storage/meta/entry/segment_entry.cpp Outdated
Comment thread src/storage/meta/entry/segment_entry.cppm Outdated
Comment thread src/storage/meta/entry/segment_entry.cppm Outdated
The BooleanT type has been replaced with bool in several places throughout multiple source files. This promotes consistency and improves readability, aligning more closely to standard C++ conventions. Changes have been applied in methods dealing with transactions, index creation, and memory management.
@JinHai-CN JinHai-CN added the ci PR can be test label Jan 10, 2024
@JinHai-CN
Copy link
Copy Markdown
Contributor

@mergify queue

@mergify
Copy link
Copy Markdown

mergify bot commented Jan 10, 2024

queue

🟠 Waiting for conditions to match

Details
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • check-success=tests / debug_tests
      • check-success=tests / release_tests and benchmark
      • #approved-reviews-by>=1
  • -draft [📌 queue requirement]
  • -mergify-configuration-changed [📌 queue -> allow_merging_configuration_change setting requirement]

@JinHai-CN JinHai-CN merged commit 6ae8b4d into infiniflow:main Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci PR can be test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants