Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GLUTEN-6997][VL] Ignore a test: cleanup file if job failed #6965

Merged
merged 4 commits into from
Aug 27, 2024

Conversation

PHILO-HE
Copy link
Contributor

What changes were proposed in this pull request?

There are 5 tests failed occasionally, all in GlutenInsertSuite.scala.

Gluten - do not remove non-v1writes sort and project *** FAILED ***
2024-08-21T09:05:09.0801992Z   org.apache.spark.SparkRuntimeException: [LOCATION_ALREADY_EXISTS] Cannot name the managed table as `spark_catalog`.`default`.`t`, as its associated location 'file:/__w/incubator-gluten/incubator-gluten/gluten-ut/spark35/target/scala-2.13/test-classes/unit-tests-working-home/spark-warehouse/t' already exists. Please pick a different table name, or remove the existing location first.

@github-actions github-actions bot added the CORE works for Gluten Core label Aug 21, 2024
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

@PHILO-HE PHILO-HE marked this pull request as ready for review August 21, 2024 14:16
@PHILO-HE PHILO-HE marked this pull request as draft August 21, 2024 14:16
Copy link

Run Gluten Clickhouse CI

withTable("t") {
spark.sql("CREATE TABLE t (c1 int, c2 string) USING PARQUET")
val table = spark.sessionState.catalog.getTableMetadata(TableIdentifier("t"))
withTable("t1") {
Copy link
Member

Choose a reason for hiding this comment

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

Does the change fix the issue?

If yes it's weird as:

  1. More withTable('t')s are in the file
  2. withTable('t') {} should remove the files after scope exits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zhztheplayer, this is not the final fix. I suspect it's this test that causes following test failure, assuming its table is not removed somehow. But seems it's not.

Copy link
Member

@zhztheplayer zhztheplayer Aug 22, 2024

Choose a reason for hiding this comment

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

Aha I missed that the PR is in draft. Please go ahead. Thanks.

Copy link

Run Gluten Clickhouse CI

Copy link

Run Gluten Clickhouse CI

1 similar comment
Copy link

Run Gluten Clickhouse CI

@PHILO-HE
Copy link
Contributor Author

PHILO-HE commented Aug 23, 2024

This described occasional failure is caused by a prior test "Cleanup staging files if job is failed" that doesn't remove table sometimes when exiting. The deeper reason is unknown. This patch is just a workaround fix.
@ulysses-you, did you encounter this issue before?

Copy link

Run Gluten Clickhouse CI

@ulysses-you
Copy link
Contributor

@PHILO-HE thank you for the fix, though I did not know why it would fail sometime..

zhztheplayer
zhztheplayer previously approved these changes Aug 23, 2024
Copy link
Member

@zhztheplayer zhztheplayer left a comment

Choose a reason for hiding this comment

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

+1 as long as it fixes the issue.

Thanks @PHILO-HE

@zhztheplayer
Copy link
Member

But CI is still failing :(

Copy link

Run Gluten Clickhouse CI

@PHILO-HE PHILO-HE changed the title [VL] Fix occasional test failure [Gluten-6997][VL] Ignore a test: cleanup file if job failed Aug 23, 2024
@PHILO-HE PHILO-HE changed the title [Gluten-6997][VL] Ignore a test: cleanup file if job failed [GLUTEN-6997][VL] Ignore a test: cleanup file if job failed Aug 23, 2024
Copy link

#6997

@PHILO-HE PHILO-HE marked this pull request as ready for review August 27, 2024 01:07
@PHILO-HE
Copy link
Contributor Author

@zhztheplayer, let's temporarily ignore this test to avoid blocking PRs. Could you approve it if it's ok to you?

@zhztheplayer
Copy link
Member

Works for me. Thank you.

@PHILO-HE PHILO-HE merged commit eff5de6 into apache:main Aug 27, 2024
45 checks passed
sharkdtu pushed a commit to sharkdtu/gluten that referenced this pull request Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE works for Gluten Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants