Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
Thank you @zhouxinyu
---
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
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
Merged, thanks @Jaskey , please help close this PR.
---
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
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
@lizhanhui @zhouxinyu @shroman
Any more advice on this pr? and when can this be merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
@Jaskey I agree that `setServiceState` should be deprecated. I overlooked
it was _public_.
---
If your project is set up for it, you can reply to this email and have your
reply appear on
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10339753/badge)](https://coveralls.io/builds/10339753)
Coverage decreased (-0.07%) to 31.449% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10339753/badge)](https://coveralls.io/builds/10339753)
Coverage decreased (-0.07%) to 31.449% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10339657/badge)](https://coveralls.io/builds/10339657)
Coverage decreased (-0.02%) to 31.504% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10339657/badge)](https://coveralls.io/builds/10339657)
Coverage decreased (-0.02%) to 31.504% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10339657/badge)](https://coveralls.io/builds/10339657)
Coverage decreased (-0.02%) to 31.504% when pulling
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
@shroman
Actully setServiceState should be synchroinzed in my opnion, say if I am
trying to shutdown, and some other thread B is trying to setServiceState ,which
should be waiting for
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
@shroman @vongosling
Do all guys consider makeing method synchrinized which is locking the
consumer object is a better way instead of a more fine-grained lock.
If so, I
Github user lizhanhui commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
As pointed out, I agree marking shutdown and start `synchronized` to keep
code as concise as possible.
---
If your project is set up for it, you can reply to this email and have your
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10329086/badge)](https://coveralls.io/builds/10329086)
Coverage increased (+0.03%) to 31.548% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10329086/badge)](https://coveralls.io/builds/10329086)
Coverage increased (+0.03%) to 31.548% when pulling
Github user shroman commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
I also think that using `synchronized` fits well here.
But I would do it in the following way:
1. Make `serviceState` volatile
```java
private volatile ServiceState
Github user zhouxinyu commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
IMO, consider that `start/shutdown` is not a high frequency usage method,
use `synchronized` may be more graceful and readable.
please @lizhanhui @shroman help review at your
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10309078/badge)](https://coveralls.io/builds/10309078)
Coverage increased (+0.1%) to 31.636% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10309014/badge)](https://coveralls.io/builds/10309014)
Coverage increased (+0.1%) to 31.627% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10308981/badge)](https://coveralls.io/builds/10308981)
Coverage increased (+0.2%) to 31.72% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10308981/badge)](https://coveralls.io/builds/10308981)
Coverage increased (+0.2%) to 31.72% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10308840/badge)](https://coveralls.io/builds/10308840)
Coverage increased (+0.3%) to 31.803% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10308840/badge)](https://coveralls.io/builds/10308840)
Coverage increased (+0.3%) to 31.803% when pulling
Github user coveralls commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
[![Coverage
Status](https://coveralls.io/builds/10308840/badge)](https://coveralls.io/builds/10308840)
Coverage increased (+0.3%) to 31.803% when pulling
Github user Ah39 commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
**# apache/curator TreeCache Start**
public TreeCache start() throws Exception
{
**Preconditions.checkState(treeState.compareAndSet(TreeState.LATENT,
Github user Jaskey commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
@Ah39
The problem is not only the synchrinaztion on state's read/write, but on
`shutdown` and `start` , `shutdown` and `start` should be syncronized
For example, two thread
Github user Ah39 commented on the issue:
https://github.com/apache/incubator-rocketmq/pull/68
1.AtomicReference State = new AtomicReference()
state.compareAndSet
2.volatile
can fix the problem
---
If your project is set up for it, you can reply to this email
26 matches
Mail list logo