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

[#4024]Refactor: Reduce unnecessary queries in catalog JDBC implementation #6540

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zhengkezhou1
Copy link
Contributor

@zhengkezhou1 zhengkezhou1 commented Feb 26, 2025

What changes were proposed in this pull request?

For queries involving more related tables, use a JOIN operation instead of executing separate queries to retrieve the results.

Why are the changes needed?

There are many unnecessary query operations, and the two tables can be directly associated to reduce these operations.

Part of: #4024

Does this PR introduce any user-facing change?

no

How was this patch tested?

unit tests & backend intergation tests

zhengkezhou1

This comment was marked as resolved.

@zhengkezhou1

This comment was marked as resolved.

@zhengkezhou1 zhengkezhou1 changed the title [#4024]refactor: Catalog storage queries using JOIN for JDBC [#4024]Refactor: Reduce unnecessary queries in catalog JDBC implementation Feb 27, 2025
@zhengkezhou1 zhengkezhou1 force-pushed the refactor-join-query-cm branch from 08f5d42 to 5a624d2 Compare February 27, 2025 06:42
@zhengkezhou1 zhengkezhou1 force-pushed the refactor-join-query-cm branch from 5a624d2 to 72a9039 Compare March 2, 2025 12:25
@zhengkezhou1 zhengkezhou1 marked this pull request as draft March 3, 2025 01:38
@zhengkezhou1 zhengkezhou1 force-pushed the refactor-join-query-cm branch from 72a9039 to 581134d Compare March 3, 2025 03:28
@zhengkezhou1 zhengkezhou1 marked this pull request as ready for review March 3, 2025 03:29
@zhengkezhou1
Copy link
Contributor Author

@yuqi1129, is our test infrastructure broken? I also encountered failures while testing locally. I think they are caused by TestContainers.

@zhengkezhou1
Copy link
Contributor Author

I tested the failing part in CI locally
https://github.com/apache/gravitino/actions/runs/13622804486/job/38080243805?pr=6540#:~:text=Run%20./gradlew%20%2DPtestMode,bash%20%2De%20%7B0%7D

There is no failure by executing independently rather than in parallel.
Screenshot from 2025-03-05 11-04-48

@yuqi1129
Copy link
Contributor

yuqi1129 commented Mar 5, 2025

I tested the failing part in CI locally apache/gravitino/actions/runs/13622804486/job/38080243805?pr=6540#:~:text=Run%20./gradlew%20%2DPtestMode,bash%20%2De%20%7B0%7D

There is no failure by executing independently rather than in parallel. Screenshot from 2025-03-05 11-04-48

This should be a network problem; let me try again.

@yuqi1129 yuqi1129 closed this Mar 5, 2025
@yuqi1129 yuqi1129 reopened this Mar 5, 2025
@zhengkezhou1 zhengkezhou1 force-pushed the refactor-join-query-cm branch from f8a67e0 to 08cb925 Compare March 6, 2025 12:47
@zhengkezhou1
Copy link
Contributor Author

@yuqi1129 I have fixed the error:https://github.com/apache/gravitino/actions/runs/13667966296/job/38212834258?pr=6540:

run CI locally with:

./gradlew test -PskipTests -PtestMode=embedded -PjdkVersion=17 -PjdbcBackend=h2 PskipDockerTests=false -x :web:web:test -x :web:integration-test:test -x :clients:client-python:test

It failed with another case:
image
It may not be a network problem because it always fails. Should we investigate this issue further?

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.

2 participants