Re: svn commit: r1740047 - in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes: group/ group/interceptors/ membership/ transport/ transport/bio/ transport/nio/

2016-04-21 Thread Christopher Schultz 
Konstantin,

On 4/20/16 5:15 AM, Konstantin Kolinko wrote:
> 2016-04-20 8:04 GMT+03:00  :
>> Author: kfujino
>> Date: Wed Apr 20 05:04:19 2016
>> New Revision: 1740047
>>
>> URL: http://svn.apache.org/viewvc?rev=1740047=rev
>> Log:
>> Change the channel field to protected.
>>
>> Modified:
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/TcpPingInterceptor.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastService.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastServiceImpl.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReceiverBase.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReplicationTransmitter.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/bio/BioReceiver.java
>> 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
>>
>> Modified: 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java?rev=1740047=1740046=1740047=diff
>> ==
>> --- 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>>  (original)
>> +++ 
>> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>>  Wed Apr 20 05:04:19 2016
>> @@ -144,11 +144,11 @@ public class ChannelCoordinator extends
>>  if ( Channel.SND_RX_SEQ==(svc & Channel.SND_RX_SEQ) ) {
>>  clusterReceiver.setMessageListener(this);
>>  if (clusterReceiver instanceof ReceiverBase) {
>> -
>> ((ReceiverBase)clusterReceiver).setChannel(getChannel());
>> +((ReceiverBase)clusterReceiver).setChannel(channel);
>>  }
>>  clusterReceiver.start();
>>  //synchronize, big time FIXME
>> -Member localMember = getChannel().getLocalMember(false);
>> +Member localMember = channel.getLocalMember(false);
>>  if (localMember instanceof StaticMember) {
>>  // static member
>>  StaticMember staticMember = (StaticMember)localMember;
>> @@ -167,7 +167,7 @@ public class ChannelCoordinator extends
>>  }
>>  if ( Channel.SND_TX_SEQ==(svc & Channel.SND_TX_SEQ) ) {
>>  if (clusterSender instanceof ReplicationTransmitter) {
>> -
>> ((ReplicationTransmitter)clusterSender).setChannel(getChannel());
>> +
>> ((ReplicationTransmitter)clusterSender).setChannel(channel);
>>  }
>>  valid = true;
>>  clusterSender.start();
> 
> []
> 
> 
> What is the reason for this change ?  Are you tying to fix some bug here?
> 
> In general, I do not like this change.
> When the code uses getters it gives us more flexibility in the future,
> allowing to change the implementation.  In ReceiverBase class there
> are a lot of private fields. Why make 'channel' a protected one?

+1 to Konstantin's review. Making channel protected is okay (but might
be unnecessary), but the accessor methods should continue to be used.

-chris

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: svn commit: r1740047 - in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes: group/ group/interceptors/ membership/ transport/ transport/bio/ transport/nio/

2016-04-20 Thread Keiichi Fujino 
2016-04-20 18:15 GMT+09:00 Konstantin Kolinko :

> 2016-04-20 8:04 GMT+03:00  :
> > Author: kfujino
> > Date: Wed Apr 20 05:04:19 2016
> > New Revision: 1740047
> >
> > URL: http://svn.apache.org/viewvc?rev=1740047=rev
> > Log:
> > Change the channel field to protected.
> >
> > Modified:
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/TcpPingInterceptor.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastService.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastServiceImpl.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReceiverBase.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReplicationTransmitter.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/bio/BioReceiver.java
> >
>  
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
> >
> > Modified:
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> > URL:
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java?rev=1740047=1740046=1740047=diff
> >
> ==
> > ---
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> (original)
> > +++
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> Wed Apr 20 05:04:19 2016
> > @@ -144,11 +144,11 @@ public class ChannelCoordinator extends
> >  if ( Channel.SND_RX_SEQ==(svc & Channel.SND_RX_SEQ) ) {
> >  clusterReceiver.setMessageListener(this);
> >  if (clusterReceiver instanceof ReceiverBase) {
> > -
> ((ReceiverBase)clusterReceiver).setChannel(getChannel());
> > +((ReceiverBase)clusterReceiver).setChannel(channel);
> >  }
> >  clusterReceiver.start();
> >  //synchronize, big time FIXME
> > -Member localMember = getChannel().getLocalMember(false);
> > +Member localMember = channel.getLocalMember(false);
> >  if (localMember instanceof StaticMember) {
> >  // static member
> >  StaticMember staticMember =
> (StaticMember)localMember;
> > @@ -167,7 +167,7 @@ public class ChannelCoordinator extends
> >  }
> >  if ( Channel.SND_TX_SEQ==(svc & Channel.SND_TX_SEQ) ) {
> >  if (clusterSender instanceof ReplicationTransmitter) {
> > -
> ((ReplicationTransmitter)clusterSender).setChannel(getChannel());
> > +
> ((ReplicationTransmitter)clusterSender).setChannel(channel);
> >  }
> >  valid = true;
> >  clusterSender.start();
>
> []
>
>
> What is the reason for this change ?  Are you tying to fix some bug here?
>
> In general, I do not like this change.
> When the code uses getters it gives us more flexibility in the future,
> allowing to change the implementation.  In ReceiverBase class there
> are a lot of private fields. Why make 'channel' a protected one?
>
>
I simply thought each channelInterceptor uses the channel instance directly.
The other fix were aligned with the ChannelInterceptorBase.
Because I do not stick to this fix, I will revert this fix later.
Thanks.



> Best regards,
> Konstantin Kolinko
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
>
> --
> Keiichi.Fujino
> 
> 
>

Re: svn commit: r1740047 - in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes: group/ group/interceptors/ membership/ transport/ transport/bio/ transport/nio/

2016-04-20 Thread Konstantin Kolinko 
2016-04-20 8:04 GMT+03:00  :
> Author: kfujino
> Date: Wed Apr 20 05:04:19 2016
> New Revision: 1740047
>
> URL: http://svn.apache.org/viewvc?rev=1740047=rev
> Log:
> Change the channel field to protected.
>
> Modified:
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/TcpPingInterceptor.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastService.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastServiceImpl.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReceiverBase.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReplicationTransmitter.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/bio/BioReceiver.java
> 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
>
> Modified: 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java?rev=1740047=1740046=1740047=diff
> ==
> --- 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>  (original)
> +++ 
> tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
>  Wed Apr 20 05:04:19 2016
> @@ -144,11 +144,11 @@ public class ChannelCoordinator extends
>  if ( Channel.SND_RX_SEQ==(svc & Channel.SND_RX_SEQ) ) {
>  clusterReceiver.setMessageListener(this);
>  if (clusterReceiver instanceof ReceiverBase) {
> -((ReceiverBase)clusterReceiver).setChannel(getChannel());
> +((ReceiverBase)clusterReceiver).setChannel(channel);
>  }
>  clusterReceiver.start();
>  //synchronize, big time FIXME
> -Member localMember = getChannel().getLocalMember(false);
> +Member localMember = channel.getLocalMember(false);
>  if (localMember instanceof StaticMember) {
>  // static member
>  StaticMember staticMember = (StaticMember)localMember;
> @@ -167,7 +167,7 @@ public class ChannelCoordinator extends
>  }
>  if ( Channel.SND_TX_SEQ==(svc & Channel.SND_TX_SEQ) ) {
>  if (clusterSender instanceof ReplicationTransmitter) {
> -
> ((ReplicationTransmitter)clusterSender).setChannel(getChannel());
> +
> ((ReplicationTransmitter)clusterSender).setChannel(channel);
>  }
>  valid = true;
>  clusterSender.start();

[]


What is the reason for this change ?  Are you tying to fix some bug here?

In general, I do not like this change.
When the code uses getters it gives us more flexibility in the future,
allowing to change the implementation.  In ReceiverBase class there
are a lot of private fields. Why make 'channel' a protected one?

Best regards,
Konstantin Kolinko

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

svn commit: r1740047 - in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes: group/ group/interceptors/ membership/ transport/ transport/bio/ transport/nio/

2016-04-19 Thread kfujino 
Author: kfujino
Date: Wed Apr 20 05:04:19 2016
New Revision: 1740047

URL: http://svn.apache.org/viewvc?rev=1740047=rev
Log:
Change the channel field to protected.

Modified:

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatchInterceptor.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/TcpPingInterceptor.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastService.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/membership/McastServiceImpl.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReceiverBase.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/ReplicationTransmitter.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/bio/BioReceiver.java

tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java?rev=1740047=1740046=1740047=diff
==
--- 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelCoordinator.java
 Wed Apr 20 05:04:19 2016
@@ -144,11 +144,11 @@ public class ChannelCoordinator extends
 if ( Channel.SND_RX_SEQ==(svc & Channel.SND_RX_SEQ) ) {
 clusterReceiver.setMessageListener(this);
 if (clusterReceiver instanceof ReceiverBase) {
-((ReceiverBase)clusterReceiver).setChannel(getChannel());
+((ReceiverBase)clusterReceiver).setChannel(channel);
 }
 clusterReceiver.start();
 //synchronize, big time FIXME
-Member localMember = getChannel().getLocalMember(false);
+Member localMember = channel.getLocalMember(false);
 if (localMember instanceof StaticMember) {
 // static member
 StaticMember staticMember = (StaticMember)localMember;
@@ -167,7 +167,7 @@ public class ChannelCoordinator extends
 }
 if ( Channel.SND_TX_SEQ==(svc & Channel.SND_TX_SEQ) ) {
 if (clusterSender instanceof ReplicationTransmitter) {
-
((ReplicationTransmitter)clusterSender).setChannel(getChannel());
+
((ReplicationTransmitter)clusterSender).setChannel(channel);
 }
 valid = true;
 clusterSender.start();
@@ -177,14 +177,14 @@ public class ChannelCoordinator extends
 membershipService.setMembershipListener(this);
 if (membershipService instanceof McastService) {
 ((McastService)membershipService).setMessageListener(this);
-((McastService)membershipService).setChannel(getChannel());
+((McastService)membershipService).setChannel(channel);
 }
 membershipService.start(MembershipService.MBR_RX);
 valid = true;
 }
 if ( Channel.MBR_TX_SEQ==(svc & Channel.MBR_TX_SEQ) ) {
 if (membershipService instanceof McastService) {
-((McastService)membershipService).setChannel(getChannel());
+((McastService)membershipService).setChannel(channel);
 }
 membershipService.start(MembershipService.MBR_TX);
 valid = true;

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java?rev=1740047=1740046=1740047=diff
==
--- 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
 (original)
+++ 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/ChannelInterceptorBase.java
 Wed Apr 20 05:04:19 2016
@@ -30,7 +30,7 @@ public abstract class ChannelInterceptor
 
 private ChannelInterceptor next;
 private ChannelInterceptor previous;
-private Channel channel;
+protected Channel channel;
 //default value, always process
 protected int optionFlag = 0;
 

Modified: 
tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/group/interceptors/MessageDispatch15Interceptor.java
URL: