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

BIGTOP-3626: Upgrade log4j dependencies for ycsb #848

Merged
merged 1 commit into from
Dec 31, 2021

Conversation

elukey
Copy link
Contributor

@elukey elukey commented Dec 28, 2021

At the time of writing upstream didn't release any official fix,
but brianfrankcooper/YCSB#1583 seems
taking care of it.

Credits for the upstream fix: Filipe Oliveira <filipecosta.90@gmail.com>

At the time of writing upstream didn't release any official fix,
but brianfrankcooper/YCSB#1583 seems
taking care of it.

Credits for the upstream fix: Filipe Oliveira <filipecosta.90@gmail.com>
@elukey
Copy link
Contributor Author

elukey commented Dec 28, 2021

The package builds fine, pending smoke tests.

$ dpkg -c ./output/ycsb/ycsb_0.17.0-1_all.deb | egrep 'log4j.*-2\..*'
-rw-r--r-- root/root    301742 2021-12-28 18:29 ./usr/lib/ycsb/elasticsearch5-binding/lib/log4j-api-2.17.0.jar
-rw-r--r-- root/root   1789297 2021-12-28 18:29 ./usr/lib/ycsb/elasticsearch5-binding/lib/log4j-core-2.17.0.jar
-rw-r--r-- root/root    218967 2021-12-28 18:29 ./usr/lib/ycsb/geode-binding/lib/log4j-api-2.7.jar
-rw-r--r-- root/root   1296829 2021-12-28 18:29 ./usr/lib/ycsb/geode-binding/lib/log4j-core-2.7.jar
-rw-r--r-- root/root     21034 2021-12-28 18:29 ./usr/lib/ycsb/ignite-binding/lib/ignite-log4j2-2.7.0.jar
-rw-r--r-- root/root    301742 2021-12-28 18:29 ./usr/lib/ycsb/ignite-binding/lib/log4j-api-2.17.0.jar
-rw-r--r-- root/root   1789297 2021-12-28 18:29 ./usr/lib/ycsb/ignite-binding/lib/log4j-core-2.17.0.jar
-rw-r--r-- root/root    122718 2021-12-28 18:29 ./usr/lib/ycsb/tablestore-binding/lib/log4j-api-2.0.2.jar
-rw-r--r-- root/root    783431 2021-12-28 18:29 ./usr/lib/ycsb/tablestore-binding/lib/log4j-core-2.0.2.jar
-rw-r--r-- root/root     22863 2021-12-28 18:29 ./usr/lib/ycsb/tablestore-binding/lib/log4j-slf4j-impl-2.0.2.jar
-rw-r--r-- root/root    301742 2021-12-28 18:29 ./usr/lib/ycsb/voltdb-binding/lib/log4j-api-2.17.0.jar
-rw-r--r-- root/root   1789297 2021-12-28 18:29 ./usr/lib/ycsb/voltdb-binding/lib/log4j-core-2.17.0.jar
-rw-r--r-- root/root     24221 2021-12-28 18:29 ./usr/lib/ycsb/voltdb-binding/lib/log4j-slf4j-impl-2.17.0.jar

Copy link
Member

@iwasakims iwasakims left a comment

Choose a reason for hiding this comment

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

+1. ran smoke-tests of ycsb on CentOS 8 x86_64.

@elukey
Copy link
Contributor Author

elukey commented Dec 29, 2021

Thanks for the review! Smoke tests passed on Debian 11 X86.

@elukey
Copy link
Contributor Author

elukey commented Dec 29, 2021

@iwasakims I have never merged a commit for Bigtop so if you could share some tips I'd be grateful. I noticed on the Wiki that we have a procedure via Gitbox, but I am wondering if simply merging from the Github UI is fine as well for the project (and if so, if there are some gotchas before doing it).

@elukey elukey merged commit 265e891 into apache:master Dec 31, 2021
@iwasakims
Copy link
Member

@elukey Sorry for late reply.

I noticed on the Wiki that we have a procedure via Gitbox, but I am wondering if simply merging from the Github UI is fine as well for the project (and if so, if there are some gotchas before doing it).

Merging PR on GitHub UI should be enough for updating master branch. I use Git CLI for cherry-picking the commit to older branches such as branch-3.0 and branch-1.5. I always use "squash and merge" for associating one commit to one JIRA issue. (Hadoop disabled other choises on GitHub site configuration.)

You should be able to commit the patch to Gitbox too since changes are synced between GitHub and Gitbox. I usually use GitHub because almost all patches are submitted via GitHub PR these days.
https://blogs.apache.org/foundation/entry/the-apache-software-foundation-expands

@elukey
Copy link
Contributor Author

elukey commented Jan 3, 2022

@iwasakims thanks a lot for the reply!

Cherry picking to branch-3.0 is something that I didn't think of, probably this change should be ported as well?

@iwasakims
Copy link
Member

@elukey Yeah, it seems to be cleanly applicable to branch-3.0. I use command lines like following for cherry-picking. It would nice if you can build and test the artifacts before pushing the branch to GitHub/Gitbox.

$ git checkout branch-3.0
$ git pull origin branch-3.0
$ git log -1 --oneline master
265e891a (origin/master, origin/HEAD, master) BIGTOP-3626: Upgrade log4j dependencies for ycsb (#848)

$ git cherry-pick -x 265e891a
...
$ git push origin branch-3.0

iwasakims pushed a commit that referenced this pull request Jan 11, 2022
At the time of writing upstream didn't release any official fix,
but brianfrankcooper/YCSB#1583 seems
taking care of it.

Credits for the upstream fix: Filipe Oliveira <filipecosta.90@gmail.com>

(cherry picked from commit 265e891)
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