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.

Sign up for GitHub

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

Jump to bottom

Add replicated storage for remote websocket subscriptions #9311

Merged
merged 28 commits into from
Apr 11, 2018
Merged

Add replicated storage for remote websocket subscriptions #9311

merged 28 commits into from
Apr 11, 2018

Conversation

mshaposhnik
Copy link
Contributor

What does this PR do?

Add JGroups-based replicated storage for remote websocket subscriptions.

What issues does this PR fix or reference?

#9014 (partially)

Release Notes

Added JGroups-based replicated storage for remote websocket subscriptions.

Docs PR

N/A

@benoitf benoitf added target/branch Indicates that a PR will be merged into a branch other than master. status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Apr 2, 2018
skabashnyuk
skabashnyuk reviewed Apr 2, 2018
View reviewed changes
Copy link
Contributor

@skabashnyuk skabashnyuk left a comment

Choose a reason for hiding this comment

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

In general Ok. @riuvshin wdyt?

@mshaposhnik
Copy link
Contributor Author

ci-test

@codenvy-ci
Copy link

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

@mshaposhnik
Copy link
Contributor Author

ci-test

@codenvy-ci
Copy link

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

dkuleshov
dkuleshov approved these changes Apr 3, 2018
View reviewed changes
Copy link

@dkuleshov dkuleshov left a comment

Choose a reason for hiding this comment

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

I see no unit tests, is that intentional?

akorneta
akorneta reviewed Apr 3, 2018
View reviewed changes
...in/java/org/eclipse/che/multiuser/api/subscription/DistributedRemoteSubscriptionStorage.java
JChannel channel = new JChannel(confFile);
this.lockService = new LockService(channel);
channel.connect(CHANNEL_NAME);
subscriptions = new ReplicatedHashMap<>(channel);
Copy link
Contributor

@akorneta akorneta Apr 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

looks like we are trying to achieve the consistent state of subscribers in each period of time, and so updates should be performed across the cluster synchronously, to achieve that you can use blocking_updates property, if we won't need that we can manage local concurrent operations using read/write locks instead of LockService.

Copy link
Contributor Author

@mshaposhnik mshaposhnik Apr 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Makes sense, added.

@mshaposhnik mshaposhnik changed the base branch from jpa_coordination to master April 11, 2018 10:46
@mshaposhnik mshaposhnik merged commit 66f9d0b into master Apr 11, 2018
@mshaposhnik mshaposhnik deleted the rpc_coordination branch April 11, 2018 11:33
@benoitf 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 benoitf added this to the 6.4.0 milestone Apr 11, 2018
@mshaposhnik mshaposhnik mentioned this pull request Apr 11, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akorneta akorneta akorneta left review comments

@skabashnyuk skabashnyuk skabashnyuk left review comments

@dkuleshov dkuleshov dkuleshov approved these changes

@riuvshin riuvshin Awaiting requested review from riuvshin

@vparfonov vparfonov Awaiting requested review from vparfonov

@sleshchenko sleshchenko Awaiting requested review from sleshchenko

Assignees
No one assigned
Labels
kind/task Internal things, technical debt, and to-do tasks to be performed. target/branch Indicates that a PR will be merged into a branch other than master.
Projects
None yet
Milestone
6.4.0
Development

Successfully merging this pull request may close these issues.
6 participants
@mshaposhnik @codenvy-ci @skabashnyuk @dkuleshov @akorneta @benoitf