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

[#6307] feat(flink): remove log4j from Gravitino Flink connector #6308

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

FANNG1
Copy link
Contributor

@FANNG1 FANNG1 commented Jan 17, 2025

What changes were proposed in this pull request?

remove log4j from Gravitino Flink connector

Why are the changes needed?

Fix: #6307

Does this PR introduce any user-facing change?

no

How was this patch tested?

setup an local flink cluster, run flink SQL

@FANNG1 FANNG1 self-assigned this Jan 17, 2025
@FANNG1 FANNG1 added the branch-0.8 Automatically cherry-pick commit to branch-0.8 label Jan 17, 2025
@FANNG1
Copy link
Contributor Author

FANNG1 commented Jan 17, 2025

@coolderli @jerryshao PTAL

@yuqi1129
Copy link
Contributor

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

@FANNG1
Copy link
Contributor Author

FANNG1 commented Jan 17, 2025

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

yes, it works with 0.7.0-inbucating

@yuqi1129
Copy link
Contributor

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

yes, it works with 0.7.0-inbucating

So I suggest we may need to find out which commit introduced the problem and make the necessary changes accordingly.

@FANNG1
Copy link
Contributor Author

FANNG1 commented Jan 17, 2025

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

yes, it works with 0.7.0-inbucating

So I suggest we may need to find out which commit introduced the problem and make the necessary changes accordingly.

I don' think so, it's reasonable to remove log4j from the Flink runtime jar to avoid the conflict.

@yuqi1129
Copy link
Contributor

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

yes, it works with 0.7.0-inbucating

So I suggest we may need to find out which commit introduced the problem and make the necessary changes accordingly.

I don' think so, it's reasonable to remove log4j from the Flink runtime jar to avoid the conflict.

I don't deny this remove log4j from the Flink runtime jar is resonable, what I emphasize is that we should know which commit introduces this problem.

@FANNG1
Copy link
Contributor Author

FANNG1 commented Jan 17, 2025

Have you identified the root cause of this issue? Does it work with 0.7.0-inbucating?

yes, it works with 0.7.0-inbucating

So I suggest we may need to find out which commit introduced the problem and make the necessary changes accordingly.

I don' think so, it's reasonable to remove log4j from the Flink runtime jar to avoid the conflict.

I don't deny this remove log4j from the Flink runtime jar is resonable, what I emphasize is that we should know which commit introduces this problem.

we didn't upgrade log4j version for one year, seems not the commit in Gravitino introduce this problems. I guess the new package changes the sequence of the classes loaded by Flink classloader introduce this problem.

@yuqi1129 yuqi1129 merged commit abe75e0 into apache:main Jan 17, 2025
25 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 17, 2025
### What changes were proposed in this pull request?

remove log4j from Gravitino Flink connector

### Why are the changes needed?

Fix: #6307 

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

### How was this patch tested?
setup an local flink cluster, run flink SQL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.8 Automatically cherry-pick commit to branch-0.8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug report] Flink sql client start failed with Graviitno Flink connector for log4j conflict
2 participants