Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on refining several internal constants and mechanisms to enhance the system's performance and resource utilization. The changes include optimizing Parquet file metadata loading, improving the file cache's capacity distribution, and aligning query execution batch sizes with DataFusion defaults for more efficient data processing. These adjustments aim to streamline operations and improve overall system responsiveness. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a set of valuable improvements focused on tuning constants and enhancing data processing logic. Key changes include adjusting Parquet read/write constants to improve performance and on-disk format stability, and refactoring several components to use the configurable session batch size, which increases consistency across the system. Additionally, the logic for splitting file cache capacity has been made more robust, fixing a potential overallocation bug. The adoption of a more modern Parquet metadata reader is also a welcome improvement. Overall, these changes are well-implemented and positively contribute to the codebase's quality and configurability.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 930f70b052
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
discord9
left a comment
There was a problem hiding this comment.
absent could impl output batch size, rest LGTM
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
| let num_rows = self.output_batch.as_ref().unwrap().num_rows(); | ||
| if num_rows == 0 { | ||
| self.output_batch_offset = 0; | ||
| return Ok(self.output_batch.take()); | ||
| } |
There was a problem hiding this comment.
Is it expected to return an empty record batch in range select?
| if self.output_timestamps.len() >= self.batch_size { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
If we return early here, do we need to update the input_timestamp_offset?
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
Adjust constants to make them more streamlined
PR Checklist
Please convert it to a draft if some of the following conditions are not met.