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

[EPIC] Container management improvement for integration tests #2765

Closed
4 tasks done
unknowntpo opened this issue Apr 1, 2024 · 4 comments
Closed
4 tasks done

[EPIC] Container management improvement for integration tests #2765

unknowntpo opened this issue Apr 1, 2024 · 4 comments
Assignees
Labels
epic Key feature

Comments

@unknowntpo
Copy link
Contributor

unknowntpo commented Apr 1, 2024

Describe the proposal

Currently, the integration tests of Gravitino uses many containers, and we want to use ContainerSuit to manage them, but there are some points that we can improve.

Current design of ContainerSuit has implemented singleton, which means no matter how many tests we are running, same kind of container (e.g. Hive) will be run at most once.

Here’s a bug, ContainerSuite.java implemented a singleton, but startHiveContainer , startTrinoContainer doesn’t determine whether container has already started or not, so this might cause multiple invokation of methods like startTrinoContainer unpredicable, these methods needs to be singleton, too, and when other test case invoke these singleton method, then needs to wait until this method is done.

Another problem is, their are some test cases that haven’t been placed in ContainerSuit, e.g. MysqlContainer in AuditCatalogMysqlIT.java, we need to put them back to ContainerSuit.

And these modification might cause a problem, because they are connected to same database concurrently, they needs to be separated to different database which name is the method name of test case. To avoid wrongly hard coded method name, we may need to get class name from Class, and extract this behavior into a method.

thanks @xunliu for pointing out these problems.

Task list

@unknowntpo unknowntpo added the epic Key feature label Apr 1, 2024
@mchades
Copy link
Contributor

mchades commented Apr 2, 2024

make methods in ContainerSuit singleton.

Do you have time to start this first recently?

@mchades
Copy link
Contributor

mchades commented Apr 3, 2024

I suspect that the failure of the Hive container to start in CI is related to it being started multiple times, so I prioritized fixing this issue by #2794

You can move on to other fixes when you have time.

BTW, I think there is another CI improvement that can be considered, which is to upload the process logs in the container (such as Hive, HDFS, etc.) after a CI failure.

@unknowntpo
Copy link
Contributor Author

unknowntpo commented Apr 3, 2024

I suspect that the failure of the Hive container to start in CI is related to it being started multiple times, so I prioritized fixing this issue by #2794

@mchades sorry for the late reply, ok, I'll move on to other issues.

@mchades
Copy link
Contributor

mchades commented Aug 9, 2024

Close this issue because it seems that all related subtasks have been completed.

@mchades mchades closed this as completed Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Key feature
Projects
None yet
Development

No branches or pull requests

3 participants