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

XDOCKER-51: Expose configuration options to configure an XWiki cluster #24

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ASHISH932
Copy link
Contributor

Setup clustering using udp
To use different channel mount volume and add configuration file

Setup clustering using udp
To use different channel mount volume and add configuration file
* Updated build.gradle to copy configuration file of jgroup
@ASHISH932
Copy link
Contributor Author

Please Review after that I would push the rest of the file generated by build.gradle

@vmassol vmassol self-assigned this Aug 19, 2019
README.md Outdated
@@ -361,6 +361,76 @@ secrets:
name: xwiki-db-root-password
```

## Configuring clustering

Read about setting communication channels [here](https://www.xwiki.org/xwiki/bin/view/Documentation/AdminGuide/Clustering/).
Copy link
Member

Choose a reason for hiding this comment

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

Small detail. Could be better to use:

Read about [setting communication channels](https://www.xwiki.org/xwiki/bin/view/Documentation/AdminGuide/Clustering/).

WDYT?

- DB_DATABASE=${DB_DATABASE}
- DB_HOST=xwiki-postgres-db
- CLUSTER=true
- CLUSTER_CHANNEL=udp
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we shouldn't prefix all XWiki-related env properties with "XWIKI", WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does XWIKI-related include variable for DB and JGroups ?
like CLUSTER above?

Copy link
Member

Choose a reason for hiding this comment

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

CLUSTER and CLUSTER_CHANNEL are XWiki properties, not jgroups one. So I would prefix with XWiki.

Now just checked and I see that we recently introduced INDEX_HOST and INDEX_PORT for solr, see
https://github.com/xwiki-contrib/docker-xwiki/blob/4ebe570a79aeed8eabf7a8b6b834a179efc4b8b1/template/xwiki/docker-entrypoint.sh#L126 and they were not prefixed with XWiki.

New idea: why not use the same property name than in the xwiki config file? That could make it simpler maybe. The properties are:

  • observation.remote.enabled
  • observation.remote.channels

This would mean also deprecating INDEX_HOST and INDEX_PORT and instead introduce a solr.remote.url property (see https://github.com/xwiki-contrib/docker-xwiki/blob/4ebe570a79aeed8eabf7a8b6b834a179efc4b8b1/template/xwiki/docker-entrypoint.sh#L149).

In this manner, we would have a good strategy for any future properties that we need to add.

WDYT?

Also, we could it more generically so that the user would be able to pass any property from xwiki.cfg or xwiki.properties using docker run -e <property name> and the entry point script would set them in xwiki.cfg or xwiki.properties. To know in which file to add, we could search the file to see if it contains the property name.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using the same property name seems good.

I think for more generic property user should bind-mount it's property.

Copy link
Member

Choose a reason for hiding this comment

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

I think for more generic property user should bind-mount it's property.

Actually I forgot that we already expose the properties files in the permanent directory so that's taken care of already. Well, except for the same reason as you're doing this PR, i.e. to be able to control it from a compose file and avoid using 2 lines of sed....

So IMO we should still implement the generic mechanism.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay we can do that, but in bash env variable naming we cannot have dot(.).
So solr.remote.url cannot be set as a variable name in bash
https://unix.stackexchange.com/questions/93532/exporting-a-variable-with-dot-in-it

So for generic mechanisim, we need a naming convention like replacing dot with dash, but they have also issues if some properties contain - in it's name

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, not easy. So we should probaby drop my idea and continue with what you did, i.e. CLUSTER and CLUSTER_CHANNEL. And for other cases, then the caller will simply need to modify the xwiki configuration files from the permanent directory.

Thanks for the discussion :)

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
build.gradle Outdated Show resolved Hide resolved
template/Dockerfile Outdated Show resolved Hide resolved
@@ -0,0 +1,82 @@

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to add license headers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know about license headers much? Should it contain xwiki license headers? As I have taken the file from JGroups ->
https://github.com/belaban/JGroups/blob/master/conf/udp.xml

Copy link
Member

Choose a reason for hiding this comment

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

hmm I thought I remembered that we were providing this xml file somewhere on xwiki.org but cannot find it. In any case we need a license header for all files we provide. We have 2 solutions:

  1. extract the default config file from the jgroups jar at execution time (on first run for example). This allows to be up to date and reduce maintenance cost.
  2. provide the file as you did and put our own license headers at the top. See https://dev.xwiki.org/xwiki/bin/view/Community/DevelopmentPractices#HForXMLfiles

I prefer option 1.

@tmortagne any opinion on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This configuration file is not exactly the same with the jgroups jar. I have changed it for docker as documented in https://github.com/belaban/jgroups-docker. So I think option 2 would be better

@vmassol
Copy link
Member

vmassol commented Aug 19, 2019

ok I've finished the review. It seems good. I've only added minor comments. Thx!

README.md Show resolved Hide resolved
@vmassol
Copy link
Member

vmassol commented Aug 19, 2019

Sounds good. I just put a minor comment on the doc and I can apply it! thx

@ASHISH932
Copy link
Contributor Author

ASHISH932 commented Aug 19, 2019

I checked again the clustering 'observation.remote.channels' is not getting set. I assumed it would. The problem which is occurring is the xwiki.properties is not following the convention.

Basically, every property that is to set is written in a commented format in xwiki.properties
eg:-
"# solr.remote.url http://localhost.com"

but in case of observation.remote.channels no such property is found. So sed cannot replace and insert the property.

@ASHISH932
Copy link
Contributor Author

How could I check that my clustering is working fine and nodes are communicating with each other, the thing I tested was creating a page in one node and accessing it on another, but I just checked that was due to shared file storage.

So how to check JGroup is properly setup?

@vmassol
Copy link
Member

vmassol commented Aug 20, 2019

So how to check JGroup is properly setup?

Hi @ASHISH932 . To test, you need to start the 2 XWiki instances (on different ports for example).
Then you navigate to a page on both instances (so that it's in the doc cache). Then on instance1
you edit this page and save it. Then you access that page on the second instance and verify if the change is there or not. If not, it means the jgroups message has not been received and the doc cache has not been invalidated for that page.

@ASHISH932
Copy link
Contributor Author

ASHISH932 commented Aug 24, 2019

I have updated and tested, now clustering is working

but as you can see in README.md docker-compose file of clustering I have to individually mount each data folder, as I have to exclude solr.

Because solr data was in the shared file system, due to which only one container can access solr.

So to use clustering we cannot keep solr data folder in shared file system

* Pushing all files after building from build.gradle
* docker-entrypoint.sh templated corrected
@ASHISH932
Copy link
Contributor Author

completed from my end.

echo "Setting cluster channel to $CLUSTER_CHANNEL"
xwiki_replace_example 'observation.remote.channels' "$CLUSTER_CHANNEL"
else
xwiki_replace_example 'observation.remote.channels' 'udp'
Copy link
Member

Choose a reason for hiding this comment

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

Why is it needed to replace an example in the config file? This feels weird :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In xwiki.properties there is no field of
"# observation.remote.channels"
So I didn't find anything to replace. So I replaced the Example

volumes:
- xwiki-data-cache:/usr/local/xwiki/data/cache
- xwiki-data-jobs:/usr/local/xwiki/data/jobs
- xwiki-data-extension:/usr/local/xwiki/data/extension
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you created 3 mounts and not just one for the perm dir. Could you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

commented below

README.md Outdated
bridge:
driver: bridge
services:
web:
Copy link
Member

Choose a reason for hiding this comment

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

Idea: use web1 since there's web2 below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay I would update that

@vmassol
Copy link
Member

vmassol commented Sep 10, 2019

I have updated and tested, now clustering is working

but as you can see in README.md docker-compose file of clustering I have to individually mount each data folder, as I have to exclude solr.

Because solr data was in the shared file system, due to which only one container can access solr.

So to use clustering we cannot keep solr data folder in shared file system

I don't understand this part. Each xwiki instance has a permdir and the solr folder contains the solr index which is remote when using a remote solr server. What is the problem? Thx

@ASHISH932
Copy link
Contributor Author

Basically, when we use internal solr, multiple solr is run on each container but they all are using the same directory. So this creates a problem, as multiple solr cannot share the repository.
Fix for this:-

  1. Use external solr
  2. do not share the solr directory

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