chore: reduce insertion size of fuzz test #4243
Conversation
|
Warning Rate limit exceeded@WenyXu has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 5 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe updates primarily involve integrating a new utility function, Changes
Sequence Diagram(s)
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
WenyXu
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests-fuzz/targets/fuzz_insert.rs (1 hunks)
- tests-fuzz/targets/fuzz_insert_logical_table.rs (3 hunks)
Additional comments not posted (4)
tests-fuzz/targets/fuzz_insert.rs (1)
67-67: LGTM!The reduction in the upper limit of the
rowsrange from 4096 to 2048 should help optimize the fuzz testing process by decreasing the volume of data being inserted during tests.tests-fuzz/targets/fuzz_insert_logical_table.rs (3)
63-63: LGTM!The addition of the
rowsfield to theFuzzInputstruct and its generation in thearbitraryimplementation are consistent with the objective of reducing the data insertion size for fuzz testing.Also applies to: 70-72
209-209: LGTM!The addition of the
rowsparameter to theinsert_valuesfunction aligns with the objective of reducing the data insertion size for fuzz testing.
262-263: LGTM!The update to pass the
rowsparameter when callinginsert_valuesaligns with the objective of reducing the data insertion size for fuzz testing.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4243 +/- ##
==========================================
- Coverage 84.84% 84.69% -0.15%
==========================================
Files 1049 1054 +5
Lines 186267 186956 +689
==========================================
+ Hits 158029 158340 +311
- Misses 28238 28616 +378 |
WenyXu
force-pushed
the
chore/reduce-fuzz-test-size
branch
from
bdd4e02 to
6d0a979
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests-fuzz/targets/fuzz_insert.rs (1 hunks)
- tests-fuzz/targets/fuzz_insert_logical_table.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests-fuzz/targets/fuzz_insert.rs
- tests-fuzz/targets/fuzz_insert_logical_table.rs
WenyXu
force-pushed
the
chore/reduce-fuzz-test-size
branch
from
6d0a979 to
1a1087e
Compare
WenyXu
requested review from
killme2008,
fengjiachun,
evenyag,
tisonkun,
CookiePieWw and
waynexia
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests-fuzz/targets/fuzz_insert.rs (1 hunks)
- tests-fuzz/targets/fuzz_insert_logical_table.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- tests-fuzz/targets/fuzz_insert.rs
- tests-fuzz/targets/fuzz_insert_logical_table.rs
|
Could you explain why we need to reduce the size of data insertion? @WenyXu |
|
It's better to be configured by environment variables, and we can configure it larger in release ci and a smaller value for every PR. |
Just want to speed up the CI. |
Good point, I'm going to add this feature. |
WenyXu
force-pushed
the
chore/reduce-fuzz-test-size
branch
from
de66b7b to
06ed793
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- .env.example (1 hunks)
- tests-fuzz/src/utils.rs (1 hunks)
- tests-fuzz/targets/failover/fuzz_failover_mito_regions.rs (2 hunks)
- tests-fuzz/targets/fuzz_alter_logical_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_alter_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_create_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_insert.rs (2 hunks)
- tests-fuzz/targets/fuzz_insert_logical_table.rs (4 hunks)
- tests-fuzz/targets/unstable/fuzz_create_table_standalone.rs (3 hunks)
Files skipped from review due to trivial changes (1)
- .env.example
Additional comments not posted (18)
tests-fuzz/targets/fuzz_create_table.rs (2)
60-61: LGTM!The use of
get_from_env_or_default_valueto retrieveGT_FUZZ_INPUT_MAX_COLUMNSwith a default value of 30 is a good addition. This makes the code more flexible and configurable.
Line range hint
73-74: LGTM!The use of
get_from_env_or_default_valueto retrieveGT_FUZZ_INPUT_MAX_COLUMNSwith a default value of 30 is consistent with the changes made in theFuzzInput::arbitraryfunction. This ensures that the number of columns is configurable via an environment variable.tests-fuzz/src/utils.rs (1)
127-136: LGTM!The function
get_from_env_or_default_valuecorrectly retrieves a value from an environment variable or returns a default value if the environment variable is not set. This function enhances the configurability of the fuzz tests.tests-fuzz/targets/fuzz_alter_table.rs (2)
73-74: LGTM!The use of
get_from_env_or_default_valueto retrieveGT_FUZZ_INPUT_MAX_COLUMNSwith a default value of 30 is a good addition. This makes the code more flexible and configurable.
43-46: LGTM!The import statements for
get_from_env_or_default_valueandGT_FUZZ_INPUT_MAX_COLUMNSare correct and necessary for the changes made in the file. These imports ensure that the code can retrieve the maximum number of columns from an environment variable.tests-fuzz/targets/fuzz_insert.rs (2)
69-72: LGTM!The use of
get_from_env_or_default_valueto retrieveGT_FUZZ_INPUT_MAX_COLUMNSandGT_FUZZ_INPUT_MAX_ROWSwith default values of 30 and 2048, respectively, is a good addition. This makes the code more flexible and configurable.
42-45: LGTM!The import statements for
get_from_env_or_default_value,flush_memtable,GT_FUZZ_INPUT_MAX_COLUMNS, andGT_FUZZ_INPUT_MAX_ROWSare correct and necessary for the changes made in the file. These imports ensure that the code can retrieve the maximum number of columns and rows from environment variables and perform the required operations.tests-fuzz/targets/fuzz_alter_logical_table.rs (2)
45-48: LGTM!The changes to import
get_from_env_or_default_valueandGT_FUZZ_INPUT_MAX_ALTER_ACTIONSare correct and align with the PR objective.
71-72: LGTM!The changes to use
get_from_env_or_default_valueforGT_FUZZ_INPUT_MAX_ALTER_ACTIONSare correct and align with the PR objective.tests-fuzz/targets/unstable/fuzz_create_table_standalone.rs (3)
47-49: LGTM!The changes to import
get_from_env_or_default_valueandGT_FUZZ_INPUT_MAX_TABLESare correct and align with the PR objective.
66-66: LGTM!The addition of the
tablesfield to theFuzzInputstruct is correct and aligns with the PR objective.
73-74: LGTM!The changes to use
get_from_env_or_default_valueforGT_FUZZ_INPUT_MAX_TABLESare correct and align with the PR objective.tests-fuzz/targets/fuzz_insert_logical_table.rs (4)
46-48: LGTM!The changes to import
get_from_env_or_default_value,GT_FUZZ_INPUT_MAX_ROWS, andGT_FUZZ_INPUT_MAX_TABLESare correct and align with the PR objective.
65-65: LGTM!The addition of the
rowsfield to theFuzzInputstruct is correct and aligns with the PR objective.
72-76: LGTM!The changes to use
get_from_env_or_default_valueforGT_FUZZ_INPUT_MAX_TABLESandGT_FUZZ_INPUT_MAX_ROWSare correct and align with the PR objective.
213-213: LGTM!The addition of the
rowsparameter to theinsert_valuesfunction call is correct and aligns with the PR objective.tests-fuzz/targets/failover/fuzz_failover_mito_regions.rs (2)
54-57: LGTM!The changes to import
get_from_env_or_default_value,GT_FUZZ_INPUT_MAX_COLUMNS,GT_FUZZ_INPUT_MAX_INSERT_ACTIONS,GT_FUZZ_INPUT_MAX_ROWS, andGT_FUZZ_INPUT_MAX_TABLESare correct and align with the PR objective.
88-95: LGTM!The changes to use
get_from_env_or_default_valueforGT_FUZZ_INPUT_MAX_COLUMNS,GT_FUZZ_INPUT_MAX_ROWS,GT_FUZZ_INPUT_MAX_TABLES, andGT_FUZZ_INPUT_MAX_INSERT_ACTIONSare correct and align with the PR objective.
WenyXu
force-pushed
the
chore/reduce-fuzz-test-size
branch
from
06ed793 to
5544797
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- .env.example (1 hunks)
- tests-fuzz/src/utils.rs (1 hunks)
- tests-fuzz/targets/failover/fuzz_failover_mito_regions.rs (2 hunks)
- tests-fuzz/targets/fuzz_alter_logical_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_alter_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_create_table.rs (2 hunks)
- tests-fuzz/targets/fuzz_insert.rs (2 hunks)
- tests-fuzz/targets/fuzz_insert_logical_table.rs (4 hunks)
- tests-fuzz/targets/unstable/fuzz_create_table_standalone.rs (3 hunks)
Files skipped from review as they are similar to previous changes (9)
- .env.example
- tests-fuzz/src/utils.rs
- tests-fuzz/targets/failover/fuzz_failover_mito_regions.rs
- tests-fuzz/targets/fuzz_alter_logical_table.rs
- tests-fuzz/targets/fuzz_alter_table.rs
- tests-fuzz/targets/fuzz_create_table.rs
- tests-fuzz/targets/fuzz_insert.rs
- tests-fuzz/targets/fuzz_insert_logical_table.rs
- tests-fuzz/targets/unstable/fuzz_create_table_standalone.rs
WenyXu
force-pushed
the
chore/reduce-fuzz-test-size
branch
from
5544797 to
034c4de
Compare
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?
Reduce data insertion size of
fuzz_insert&fuzz_insert_logical_tabletargets.Checklist
Summary by CodeRabbit
New Features
Refactor
Chores