Skip to content

Conversation

@avamingli
Copy link
Contributor

Since commit 5acbf4e, when enable_parallel is off, we will insert
into only one AO segfile even gp_appendonly_insert_files is > 1.
It will make data skew after user set enable_parallel to off,
while there are a lot of data inserted later or there are
online-steaming ETL severs.
And that make our parallel plan has a bottleneck.
In general, we should try to make AO segfiles as much as GUC
gp_appendonly_insert_files and avoid data skew for users,
no matter users use parallel query or not.
And only keep gp_appendonly_insert_files default value to 4 is enough.
Remove enable_parallel when initing AO/AOCS insertDesc.

Authored-by: Zhang Mingli [email protected]

closes: #39


Change logs

Describe your change clearly, including what problem is being solved or what feature is being added.

If it has some breaking backward or forward compatibility, please clary.

Why are the changes needed?

Describe why the changes are necessary.

Does this PR introduce any user-facing change?

If yes, please clarify the previous behavior and the change this PR proposes.

How was this patch tested?

Please detail how the changes were tested, including manual tests and any relevant unit or integration tests.

Contributor's Checklist

Here are some reminders before you submit the pull request:

  • Document changes
  • Communicate in the GitHub Issues or Discussions (list them if needed)
  • Add tests for the change
  • Pass make installcheck
  • Pass make -C src/test installcheck-cbdb-parallel

@avamingli
Copy link
Contributor Author

The hack way gpstop -u should be removed if our CI has set GUC gp_appendonly_insert_files=0 when start the cluster each time.

@avamingli avamingli force-pushed the fix_ao_segfiles branch 2 times, most recently from a662ecf to 39fdb57 Compare July 28, 2023 02:56
Since commit 5acbf4e, when enable_parallel is off, we will insert
into only one AO segfile even gp_appendonly_insert_files is > 1.
It will make data skew after user set enable_parallel to off,
while there are a lot of data inserted later or there are
online-steaming ETL severs.
And that make our parallel plan has a bottleneck.
In general, we should try to make AO segfiles as much as GUC
gp_appendonly_insert_files and avoid data skew for users,
no matter users use parallel query or not.
And only keep gp_appendonly_insert_files default value to 4 is enough.
Remove enable_parallel when initing  AO/AOCS insertDesc.

Authored-by: Zhang Mingli [email protected]
Utility mode should always use segfile 0 for AO/AOCS tables.
Insert multiple files in Utility mode will cause isolation2
fail when there are concurrent insertion like 0U to launch
processes.

Authored-by: Zhang Mingli [email protected]
We have changed gp_appendonly_insert_files to 4 by default.
It will cause lots of cases fail in cases related to AO/AOCS
segfile and etc.

As we enable insert multiple files regardless of enable_parallel,
the problems come up.

Use gpconfig to set GUCs before test cases to disable insert
multiple files and etc.

Authored-by: Zhang Mingli [email protected]
@avamingli avamingli requested a review from yjhjstz July 31, 2023 08:38
Copy link
Member

@yjhjstz yjhjstz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@avamingli avamingli merged commit 320bc30 into apache:main Jul 31, 2023
@avamingli avamingli deleted the fix_ao_segfiles branch July 31, 2023 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Enable AO/AOCO insert to multiple files even enable_parallel is off

3 participants