Skip to content

Conversation

@LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Jun 9, 2023

What changes were proposed in this pull request?

This pr aims to add System.gc at beforeEach in PruneFileSourcePartitionsSuite to avoid sql-others GA testing task OOM.

Why are the changes needed?

Make GA testing stable

Does this PR introduce any user-facing change?

No, just for test

How was this patch tested?

Should monitor CI

@LuciferYang LuciferYang marked this pull request as draft June 9, 2023 08:48
@LuciferYang LuciferYang changed the title [DRAFT] Test remove script [DRAFT] Test add System.gc to PruneFileSourcePartitionsSuite Jun 11, 2023

class PruneFileSourcePartitionsSuite extends PrunePartitionSuiteBase with SharedSparkSession {

override def beforeEach(): Unit = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's check if this is really useful @HyukjinKwon @zhengruifeng

@LuciferYang LuciferYang marked this pull request as ready for review June 12, 2023 03:01
@LuciferYang
Copy link
Contributor Author

The latest rebase did not actually run GA test, let me trigger it again

@LuciferYang LuciferYang changed the title [DRAFT] Test add System.gc to PruneFileSourcePartitionsSuite [SPARK-44023][SQL][TESTS] Test add System.gc to PruneFileSourcePartitionsSuite Jun 12, 2023
@LuciferYang LuciferYang changed the title [SPARK-44023][SQL][TESTS] Test add System.gc to PruneFileSourcePartitionsSuite [SPARK-44023][SQL][TESTS] Add System.gc at beforeEach in PruneFileSourcePartitionsSuite Jun 12, 2023
@zhengruifeng
Copy link
Contributor

it seems sql - other was already timeout,

what about figuring out the problematic test suites to disable for now?

@LuciferYang
Copy link
Contributor Author

The running one should be org.apache.spark.sql.execution.command.v2.AlterTableRenamePartitionSuite

@LuciferYang
Copy link
Contributor Author

2023-06-12T07:47:10.5042643Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32mAlterTableRenamePartitionSuite:�[0m�[0m
2023-06-12T07:47:10.7210341Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: rename without explicitly specifying database (171 milliseconds)�[0m�[0m
2023-06-12T07:47:10.7333258Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: table to alter does not exist (13 milliseconds)�[0m�[0m
2023-06-12T07:47:10.7959978Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: partition to rename does not exist (62 milliseconds)�[0m�[0m
2023-06-12T07:47:10.8771267Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: target partitions exist (80 milliseconds)�[0m�[0m
2023-06-12T07:47:10.9971419Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: single part partition (119 milliseconds)�[0m�[0m
2023-06-12T07:47:11.1922848Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: multi part partition (195 milliseconds)�[0m�[0m
2023-06-12T07:47:11.3404610Z �[0m[�[0m�[0minfo�[0m] �[0m�[0m�[32m- ALTER TABLE .. RENAME PARTITION using V2 catalog V2 command: partition spec in RENAME PARTITION should be case insensitive (147 milliseconds)�[0m�[0m

hang at org.apache.spark.sql.execution.command.v2.AlterTableRenamePartitionSuite, ignore it and re-testing

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jun 12, 2023

Another frequently killed is JavaRowSuite, but I don't think there are any issues with it.

How about downgrade sbt to 1.8.3 for more test? Ignore this

@LuciferYang
Copy link
Contributor Author

LuciferYang commented Jun 12, 2023

https://github.com/apache/spark/pull/41552/files is making another try, add a new test group to the sql module to alleviate the workload of group sql others

* The class contains tests for the `ALTER TABLE .. RENAME PARTITION` command
* to check V2 table catalogs.
*/
@Ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although GA passed, I am not sure we really need ignore this case ...

Copy link
Member

Choose a reason for hiding this comment

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

Let's ignore it for now.

@HyukjinKwon
Copy link
Member

Merged to master.

@panbingkun
Copy link
Contributor

https://github.com/apache/spark/pull/41552/files is making another try, add a new test group to the sql module to alleviate the workload of group sql others

Perhaps this is a good idea.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you for all your efforts about sql - others failures.
Although the master branch still fails after merging this, I believe this PR helps us narrow-down the investigation scope.

I'm also +1 for adding a new test group and splitting sql - others into two groups. Thank you again all.

MaxGekk pushed a commit that referenced this pull request Jun 18, 2023
…TableRenamePartitionSuite`

### What changes were proposed in this pull request?
#41533 ignore `AlterTableRenamePartitionSuite` try to restore stability of `sql-others` test task, but it seems that it is not the root cause that affects stability, so this pr has removed the previously added `ignore` identifier to restore testing.

### Why are the changes needed?
Resume testing of `AlterTableRenamePartitionSuite`

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
should monitor ci

Closes #41647 from LuciferYang/SPARK-44089.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
czxm pushed a commit to czxm/spark that referenced this pull request Jun 19, 2023
…leSourcePartitionsSuite`

### What changes were proposed in this pull request?
This pr aims to add `System.gc` at `beforeEach` in `PruneFileSourcePartitionsSuite` to avoid `sql-others` GA testing task OOM.

### Why are the changes needed?
Make GA testing stable

### Does this PR introduce _any_ user-facing change?
No, just for test

### How was this patch tested?
Should monitor CI

Closes apache#41533 from LuciferYang/GA-test.

Lead-authored-by: yangjie01 <[email protected]>
Co-authored-by: YangJie <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
czxm pushed a commit to czxm/spark that referenced this pull request Jun 19, 2023
…TableRenamePartitionSuite`

### What changes were proposed in this pull request?
apache#41533 ignore `AlterTableRenamePartitionSuite` try to restore stability of `sql-others` test task, but it seems that it is not the root cause that affects stability, so this pr has removed the previously added `ignore` identifier to restore testing.

### Why are the changes needed?
Resume testing of `AlterTableRenamePartitionSuite`

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
should monitor ci

Closes apache#41647 from LuciferYang/SPARK-44089.

Authored-by: yangjie01 <[email protected]>
Signed-off-by: Max Gekk <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants