search

scylladb / scylla-jmx

Scylla JMX proxy
GNU Affero General Public License v3.0
28 stars 51 forks source link

dist/redhat: support jre 11 instead of jre 8 #197

Closed tchaikov closed 1 year ago

tchaikov commented 1 year ago

this change fixes the dependency required by scylla-jmx's rpm package.

it is part of the series preparing for jre 11. as we are observing crashes in JVM provided by OpenJDK 8, see https://github.com/scylladb/scylla-jmx/pull/193. since OpenJDK 8 is aged and only security bugs are accepted now. let's move over to OpenJDK 11 for a better supported Java platform. since we've already changed jdk8 to jdk 11 in https://github.com/scylladb/scylla-jmx/commit/19097f2d73d0c42e60f35d3d1fa4d231cfd4809b. and install-dependencies.sh is used both for preparing developer's environment and for setting up the dbuild docker image, which is used for running dtest also in our CI. we need to update scylla-jmx's rpm accordingly, otherwise the rpm package won't install in the container setup using the dbuild image once it is updated with the latest scylla-jmx's install-dependencies.sh. please note, debian/control already depends on openjdk-11-jre as an alternative of openjdk-8-jre, so we are fine.

please note, jre-11-headless is provided by java-11-openjdk-headless.

tchaikov commented 1 year ago

@nyh hi, Nadav, i think we also need this change as a part of the plan in https://github.com/scylladb/scylla-jmx/pull/193#discussion_r1148516582

tchaikov commented 1 year ago

@avikivity thanks for queuing the change to bump up jmx, but i think we still need this one.

nyh commented 1 year ago

Merged. @tchaikov please prepare a PR for scylla.git updating the scylla-jmx submodule (there is `scripts/refresh-submodules.sh tools/jmx to help you do that). This will also allow you or the CI to test that this change actually works in the big picture.

avikivity commented 1 year ago

@nyh It's better to ask for this before merging (the PR can then be closed as it will refer to the wrong repository).

Or you can open it yourself and avoid the extra back-and-forth latency.

tchaikov commented 1 year ago

@nyh hi Nadav, thanks. https://github.com/scylladb/scylladb/pull/13356 was created to pick up the latest jmx master HEAD.

tchaikov commented 1 year ago

@nyh It's better to ask for this before merging (the PR can then be closed as it will refer to the wrong repository).

i am confused. how can we create a pull request bumping up a submodule which is supposed to include a change without getting that change landed in the submodule's repo? i think i don't have the write access to scylladb/scylla-jmx repo, and, in scylladb/scylladb repo, it references the scylla-jmx using ../scylla-jmx, so, IIUC, we need to merge this change first, no?

nyh commented 1 year ago

@nyh It's better to ask for this before merging (the PR can then be closed as it will refer to the wrong repository).

The author should have done something like this, manually, to test what he did in the submodule and how it fits the whole project.

But I think there is a limit to how pedantic we need to be. If something wrong was committed to the scylla-jmx submodule, it's not a disaster - it only becomes a disaster when the submodule is upgrade in scylla.git - and this part will be checked by CI.

Or you can open it yourself and avoid the extra back-and-forth latency.

I prefer not to. If there's a problem seen in the result, I don't want to fix it myself, I want the original submodule-patch author to worry about it.

avikivity commented 1 year ago

So it's best to ask the author for a CI run. I want to avoid a window where the submodule is not synchronized. First, it creates conversations like this. Second, what if another patch to scylla-tools-java comes in? Things start to pile up.