Configure JPA L2 cache coordination between instances on Openshift® / Kubernetes #8982

mshaposhnik · 2018-03-02T08:39:16Z

What does this PR do?

Adds JGroups based coordination for EclipseLink L2 caches for multiple master instances on openshift or K8S infra.

What issues does this PR fix or reference?

#8922

Release Notes

Added JGroups based coordination for EclipseLink L2 caches for multiple master instances on openshift or K8S infra.

Docs PR

N/A

sleshchenko · 2018-03-02T09:38:00Z

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/jgroups/che-tcp.xml

+    thread_pool.keep_alive_time="30000" />
+
+  <kubernetes.KUBE_PING
+    namespace="${OPENSHIFT_KUBE_PING_NAMESPACE:eclipse-che}"


As far as I understand it is used for Kubernetes and OpenShift infrastructures. So let's rename env var to KUBERNETES_KUBE_PING_NAMESPACE, or maybe even CHE_INFRA_KUBERNETES_KUBE__PING__NAMESPACE to make it clear that it is infrastructure specific configuration property.

I think we can remove it here at all. But we still should pass it to container as is:
See
https://github.com/jgroups-extras/jgroups-kubernetes/blob/0.9.3/kube/src/main/java/org/jgroups/ping/kube/KubePing.java#L96
+
https://github.com/jgroups-extras/jgroups-kubernetes/blob/0.9.3/kube/src/main/java/org/jgroups/ping/kube/KubePing.java#L131
I will try to remove it from here.

Up to you, but I'd define our own documented property into che.properties and che.env.

For me it seems we can't use own prop-s here.

Can you describe why we can't? Is not it just name of environment property that should be respected?

Checked the jgroups code. We can use custom system property. But env.variable should be named as in the links above.

sleshchenko · 2018-03-02T09:39:58Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -64,6 +68,8 @@ items:
            name: http
          - containerPort: 8000
            name: http-debug
+          - containerPort: 8888


Is it default value? Is it a different property from TCP_PORT?

Yes, thats different one needed for KUBE_PING.

@mshaposhnik can you elaborate on how it is used? It is not clear for me.

Pls read https://github.com/jgroups-extras/jgroups-kubernetes/tree/0.9.3

Yes, I read the docs. But I mean kubernetes service. Without it there is no access to a port of a pod.

I think there is access if you know IP for a container. I think Kubernetes JGroups discovers pods/containers IPs itself.
If you access by service, actually you don't know which pod are you request if replication is more than 1. So, it's impossible to use service at all.
Maybe I'm wrong somewhere, but I imagine it in this way.

Yeah, agree about service.

But how the lib accesses k8s/OS API? which token or SA is used?

You can set it into ENV or default will be used from here:
https://github.com/jgroups-extras/jgroups-kubernetes/blob/0.9.3/kube/src/main/java/org/jgroups/ping/kube/KubePing.java#L88

Then we need to be sure that this approach works in both kubernetes and openshift before the merge.

sleshchenko · 2018-03-02T10:28:54Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -48,6 +48,10 @@ items:
      spec:
        containers:
        - env:
+          - name: OPENSHIFT_KUBE_PING_NAMESPACE


Shouldn't be added the same changes to https://github.com/eclipse/che/blob/master/dockerfiles/init/modules/kubernetes/files/che-kubernetes.yaml?
Also maybe kubernetes helm plugin should be updated accordingly.

@garagatyi @l0rd I assume it may affect minishift addon, should it be updated or the corresponding issue created?

I can't find where this env var is used. @mshaposhnik @sleshchenko can you guys point me to it?

Pls read https://github.com/jgroups-extras/jgroups-kubernetes/tree/0.9.3

garagatyi · 2018-03-03T07:37:10Z

assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java

-  private void configureMultiUserMode() {
+  private void configureMultiUserMode(
+      Map<String, String> persistenceProperties, String infrastructure) {
+    if (OpenShiftInfrastructure.NAME.equals(infrastructure)


This hardcoding of infrastructure names makes Che less extensibility friendly. I would recommend using another approach where Che deployment can specify environment variable to configure JGROUPS. @mshaposhnik @skabashnyuk @sleshchenko WDYT?

There is a lot of ohter places where it is configured in a such way. It is not easy to refactor them all. Probably it's another big issue.

@garagatyi how do you propose to implement in another way?

@skabashnyuk having an environment variable that enables what now is enabled by the name of the active infrastructure.

garagatyi · 2018-03-03T07:38:53Z

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/jgroups/che-tcp.xml

+-->
+<config xmlns="urn:org:jgroups"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="urn:org:jgroups http://www.jgroups.org/schema/JGroups-3.1.xsd">


Can you elaborate on whether there is a way to tweak this configuration in case service delivery team need to change something?

We can probably move filename into property. Then, changing conf will be as simple as put new xml conf file under classpath and change that property. That's a good point.

Or no... that's already module. Need to find out way. Maybe discuss with @riuvshin

I see, thank you.

garagatyi · 2018-03-03T07:41:13Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -48,6 +48,10 @@ items:
      spec:
        containers:
        - env:
+          - name: OPENSHIFT_KUBE_PING_NAMESPACE


I can't find where this env var is used. @mshaposhnik @sleshchenko can you guys point me to it?

garagatyi · 2018-03-03T07:42:15Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -64,6 +68,8 @@ items:
            name: http
          - containerPort: 8000
            name: http-debug
+          - containerPort: 8888


@mshaposhnik can you elaborate on how it is used? It is not clear for me.

garagatyi

@mshaposhnik Is this already tested on a real cluster of it is just a POC?

garagatyi · 2018-03-03T09:34:47Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -64,6 +68,8 @@ items:
            name: http
          - containerPort: 8000
            name: http-debug
+          - containerPort: 8888


@mshaposhnik If there is no service for this port how this port can be accessed from other pods?

garagatyi · 2018-03-03T09:34:56Z

dockerfiles/init/modules/openshift/files/scripts/che-openshift.yml

@@ -48,6 +48,10 @@ items:
      spec:
        containers:
        - env:
+          - name: OPENSHIFT_KUBE_PING_NAMESPACE


garagatyi · 2018-03-03T09:36:06Z

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/jgroups/che-tcp.xml

+-->
+<config xmlns="urn:org:jgroups"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="urn:org:jgroups http://www.jgroups.org/schema/JGroups-3.1.xsd">


I see, thank you.

garagatyi · 2018-03-03T09:37:28Z

assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java

-  private void configureMultiUserMode() {
+  private void configureMultiUserMode(
+      Map<String, String> persistenceProperties, String infrastructure) {
+    if (OpenShiftInfrastructure.NAME.equals(infrastructure)


@skabashnyuk having an environment variable that enables what now is enabled by the name of the active infrastructure.

mshaposhnik · 2018-03-06T11:03:20Z

Checked it works on K8S:

JGroups set connection between pods:
Cache propagation :
https://drive.google.com/file/d/1aXkfi7LxR0pkYsQhOson6OaCguri_-w6/view?usp=sharing

garagatyi · 2018-03-06T11:37:18Z

@mshaposhnik Can you also check on Openshift?

mshaposhnik · 2018-03-06T13:30:38Z

It was developed on Openshift, because it is a primary goal of the issue. Kubernetes is just like an free bonus :)

garagatyi · 2018-03-06T14:02:01Z

@mshaposhnik thank you for the clarification!

sleshchenko

LGTM

It would be better to name environment configuration as Infrastructure specific property, but I'm OK to merge it as it is because looks like there is no a way to define custom env variable name for kubernetes-jgroups library.

sleshchenko · 2018-03-07T09:44:53Z

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/jgroups/che-tcp.xml

+  <MFC max_credits="2M"
+    min_threshold="0.4"/>
+  <FRAG2 frag_size="60K"/>
+  <!--RSVP resend_interval="2000" timeout="10000"/-->


Do we need this commented configuration line?

garagatyi · 2018-03-20T20:26:54Z

@mshaposhnik I'm sorry but I've merged PR that moves deployment files to another folder. This has affected your PR. I think that the easiest way to adapt your PR is to rebase it against master and then move files that would be left from dockerfiles/init/modules/che-kubernetes-helm to deploy/kubernetes/helm/che. I hope you won't get any rebase conflicts.

mshaposhnik · 2018-04-11T08:34:56Z

ci-test

codenvy-ci · 2018-04-11T14:39:31Z

ci-test build report:
Build details
Test report
selenium tests report data
docker image: eclipseche/che-server:8982
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

mshaposhnik added 2 commits March 1, 2018 15:14

Add JPA cache coordination using jgroups;

a3cf312

Revert logging level

6018351

mshaposhnik requested review from benoitf, riuvshin and skabashnyuk as code owners March 2, 2018 08:39

mshaposhnik requested a review from sleshchenko March 2, 2018 08:39

benoitf added the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Mar 2, 2018

sleshchenko reviewed Mar 2, 2018

View reviewed changes

fixup! Revert logging level

6290fb5

garagatyi reviewed Mar 3, 2018

View reviewed changes

Merge branch 'master' into jpa_coordination

97829b4

benoitf added the kind/task Internal things, technical debt, and to-do tasks to be performed. label Mar 6, 2018

mshaposhnik added 2 commits March 7, 2018 11:10

Add kubernetes changes

c31741b

Add helm plugin changes

840bd0f

sleshchenko approved these changes Mar 7, 2018

View reviewed changes

Remove commented conf part

cc2b23d

skabashnyuk mentioned this pull request Mar 7, 2018

K8s multiuser helm #8973

Merged

Merge branch 'master' into jpa_coordination

04a276c

riuvshin approved these changes Mar 14, 2018

View reviewed changes

garagatyi mentioned this pull request Mar 20, 2018

Move deployment files out of dockerfiles folder #9143

Merged

skabashnyuk approved these changes Mar 21, 2018

View reviewed changes

Merge branch 'master' into jpa_coordination

195f2ff

mshaposhnik requested a review from vparfonov as a code owner March 21, 2018 10:08

Merge branch 'master' into jpa_coordination

aca5188

mshaposhnik requested a review from a user March 28, 2018 09:11

mshaposhnik requested a review from l0rd as a code owner March 28, 2018 09:11

ghost approved these changes Mar 28, 2018

View reviewed changes

mshaposhnik added 2 commits April 2, 2018 16:14

Merge branch 'master' into jpa_coordination

54ef979

merge woth master

5cd39b3

sleshchenko mentioned this pull request Apr 10, 2018

Implemented recovery functionality for Kubernetes/OpenShift infrastructures #9301

Merged

Merge with master

f8e9d93

mshaposhnik merged commit 161ff94 into master Apr 11, 2018

benoitf removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Apr 11, 2018

benoitf added this to the 6.4.0 milestone Apr 11, 2018

mshaposhnik deleted the jpa_coordination branch April 11, 2018 10:53

Hailongli88 mentioned this pull request Apr 1, 2019

K8S:coordination #13027

Closed

Configure JPA L2 cache coordination between instances on Openshift® / Kubernetes #8982

Configure JPA L2 cache coordination between instances on Openshift® / Kubernetes #8982

Conversation

mshaposhnik commented Mar 2, 2018 • edited Loading

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshaposhnik Mar 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garagatyi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshaposhnik commented Mar 6, 2018 • edited Loading

garagatyi commented Mar 6, 2018

mshaposhnik commented Mar 6, 2018

garagatyi commented Mar 6, 2018

sleshchenko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garagatyi commented Mar 20, 2018

mshaposhnik commented Apr 11, 2018

codenvy-ci commented Apr 11, 2018

mshaposhnik commented Mar 2, 2018 •

edited

Loading

mshaposhnik Mar 3, 2018 •

edited

Loading

mshaposhnik commented Mar 6, 2018 •

edited

Loading