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

[WFLY-410] Custom stack resource for JGroups #4796

Closed
wants to merge 3 commits into from

Conversation

rachmatowicz
Copy link
Contributor

This PR does the following:

  • implements a custom Stack resource for the JGroups subsystem which maintains protocol order by returning SortedSets of children instead of Sets
  • this obviates the need for a separate PROTOCOLS attribute to maintain order as in legacy versions

@wildfly-ci
Copy link

Build 151 is now running using a merge of 9d999dfc8d31a4451e238d736c6070394eb982e6

@wildfly-ci
Copy link

Build 151 outcome was SUCCESS using a merge of 9d999dfc8d31a4451e238d736c6070394eb982e6
Summary: Tests passed: 3963, ignored: 89 Build time: 1:14:25

@jaikiran
Copy link
Contributor

jaikiran commented Aug 7, 2013

Just going to trigger a run to see if this is still mergeable.

retest this please.

@wildfly-ci
Copy link

Build 364 is now running using a merge of 9d999dfc8d31a4451e238d736c6070394eb982e6

@wildfly-ci
Copy link

Build 364 outcome was FAILURE using a merge of 9d999dfc8d31a4451e238d736c6070394eb982e6
Summary: Tests passed: 3813, ignored: 86; exit code 1 (new) Build time: 1:29:25

@wildfly-ci
Copy link

Build 366 is now running using a merge of 1cd8a73

@rachmatowicz
Copy link
Contributor Author

Re-based against master and running again.

@wildfly-ci
Copy link

Build 366 outcome was FAILURE using a merge of 1cd8a73
Summary: Tests passed: 3813, ignored: 86; exit code 1 Build time: 1:30:14

@@ -0,0 +1,130 @@
package org.jboss.as.clustering.jgroups.subsystem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing copyright header.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@bstansberry
Copy link
Contributor

I think this will break in a managed domain because when a slave HC registers it gets shipped the domain config from the master as a List and then the slave HC converts those into Resources. And a standard Resource impl is used, so your custom resource will be lost.

The slave HC uses custom Resource impls it knows about, but it has no way to know about subsystem resource impls.

This is a general issue with domain mode that I hope to address in the next week or so. A fix is needed for the blocker RBAC feature so it has to happen. So I prefer to hold off on merging this until that's in.

Sorry for not thinking of this before.

@rachmatowicz
Copy link
Contributor Author

No problem on the delay. I have more custom resources besides this one :-)
.

@wildfly-ci
Copy link

Build 378 is now running using a merge of bdce8a8

@wildfly-ci
Copy link

Build 378 outcome was SUCCESS using a merge of bdce8a8
Summary: Tests passed: 4004, ignored: 89 Build time: 1:23:18

@wildfly-ci
Copy link

Build 169 is now running using a merge of bdce8a8

@wildfly-ci
Copy link

Build 169 outcome was SUCCESS using a merge of bdce8a8
Summary: Tests passed: 4338, ignored: 82 Build time: 1:0:36

@jaikiran
Copy link
Contributor

This has been around for a while. Just triggering a run to see if this needs a rebase.

retest this please

@wildfly-ci
Copy link

Build 724 is now running using a merge of bdce8a8

@wildfly-ci
Copy link

Build 724 outcome was SUCCESS using a merge of bdce8a8
Summary: Tests passed: 4887, ignored: 88, muted: 1 Build time: 1:54:42

@rachmatowicz
Copy link
Contributor Author

This PR has been languishing for 3 months ... can we get it merged?

@bstansberry
Copy link
Contributor

I have to write the thing to let it work in a managed domain. :(

I'm still stuck in EAP 6.2 mode.

@rachmatowicz
Copy link
Contributor Author

Argh! My fault - I now see the comment above which I remember reading but just forgot about!

@bstansberry
Copy link
Contributor

Nah; I wanted to get that done a long time ago. Unfortunately the easy solution I had in mind was bogus so now I need to do it properly.

@n1hility n1hility added hold and removed hold labels Jul 28, 2014
@bstansberry
Copy link
Contributor

Closing this, as Paul went another way with #6992. Sorry it's taken so long to get the domain mode thing that was blocking this done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold PR should not be merged for some reason.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants