You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by shroman <gi...@git.apache.org> on 2017/03/09 04:37:57 UTC
[GitHub] incubator-rocketmq pull request #73: [ROCKETMQ-135] Broker cannot be properl...
GitHub user shroman opened a pull request:
https://github.com/apache/incubator-rocketmq/pull/73
[ROCKETMQ-135] Broker cannot be properly finalized on failure to load\u2026
\u2026 a storage plugin.
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-135
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/shroman/incubator-rocketmq ROCKETMQ-135
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-rocketmq/pull/73.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #73
----
commit f3ce58b74d164860abdb191c9816c03edd4a9278
Author: shtykh_roman <rs...@yahoo.com>
Date: 2017-03-09T04:36:21Z
[ROCKETMQ-135] Broker cannot be properly finalized on failure to load a storage plugin.
JIRA issue: https://issues.apache.org/jira/browse/ROCKETMQ-135
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
Folks, `MessageStoreFactory#build` returns only one store instance. Does it make any sense to iterate over the list of plugins in the configuration?
How about removing that `for` loop and just try to instantiate from the trimmed plugin string?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
[](https://coveralls.io/builds/10513484)
Coverage increased (+0.04%) to 31.027% when pulling **dde713c5d9ee0269a10befb32057363248ba70c1 on shroman:ROCKETMQ-135** into **e3f4251c91a73f4e51732bcb1690554ac5fb3096 on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@shroman
Sorry, I still cannot see the problem. No initialization are called inside the build method and only the outer class are returned.
Do you concern about the properties ways or the code implementations or the functionality?
If you are concerning about the property way, may be we can discuss more about it.
But supportive to list of plugins instead of only one will help user easily to pile up their plugin components.
As to the decorator pattern way, for example , user has three decorator plugins say DecoratedClassA, DecoratedClassB, DecoratedClassC,
when we write code we must write something like:
MessageStore c = //something here;
DecoratedClassA c1 = new DecoratedClassA (c );
DecoratedClassB c2 = new DecoratedClassB (c1 );
DecoratedClassC c3 = new DecoratedClassB (c2 );
and we just use c3 to do our jobs, which is exactly the same way as the current implementations except that rocketmq plugins have to use reflection to load class.
And if only one plugin is supported, the decorator has to be done by the developers, which is not friendly enough in my opinion
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@vongosling @zhouxinyu @lizhanhui
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
I would rather get suggestions about this modification, if any, instead of hypothetical considerations. What is described here cannot be done with the current RocketMQ code base -- it's not just config changes, you will need to restart the broker cluster. In addition, from the code I cannot see that more than one store at a time can be supported. Can anyone confirm?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@Jaskey For a decorated class, you don't write "DecoratedClassA, DecoratedClassB, DecoratedClassC" in your properties, do you? :) You just specify your outer class, say "DecoratedClassC", and initialize. Inheritance and composition are done for you.
The current code explicitly initializes all classes, and use only the latest one.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@shroman , indeed it works.But this can be.done by rocketmq actually to make it easier to make up.everything.
Say user may have to plugin two store pligins , one for mysql, one for kafka . And after days, the kafka plugin is no longer needed for a period of time, they just modify the property file to remove that plugin and the jobs will be done by rocketmq. If only one plugin is supported the need to modify the source code to provide a.new plugin don't they\uff1f
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@shroman user may have several plugins as Decorator Patterns, and `build` method returns the outtest store decorator, which in my opinion is right.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by shroman <gi...@git.apache.org>.
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@Jaskey For this, write your decorated code (what you write above) in, say, `MyDecoratedClass` and specify "MyDecoratedClass" as your storage to initialize. That will suffice.
It's up to the user how he/she implements the store plugin class (through decorators or whatever), what we do is just instantiate it from the class name.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by Jaskey <gi...@git.apache.org>.
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
@shroman
From the existing implementations, the author are apparently try to support more than one plugin easily.
Maybe we can discuss about this with a separate issue in the mail list, for now we just focus on the bug here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-rocketmq issue #73: [ROCKETMQ-135] Broker cannot be properly final...
Posted by coveralls <gi...@git.apache.org>.
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/73
[](https://coveralls.io/builds/10519936)
Coverage increased (+0.1%) to 31.087% when pulling **dde713c5d9ee0269a10befb32057363248ba70c1 on shroman:ROCKETMQ-135** into **e3f4251c91a73f4e51732bcb1690554ac5fb3096 on apache:develop**.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---