Re: svn commit: r1740047 - in /tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes: group/ group/interceptors/ membership/ transport/ transport/bio/ transport/nio/
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&view=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&r1=1740046&r2=1740047&view=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 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&view=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&r1=1740046&r2=1740047&view=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 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&view=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&r1=1740046&r2=1740047&view=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/
Author: kfujino Date: Wed Apr 20 05:04:19 2016 New Revision: 1740047 URL: http://svn.apache.org/viewvc?rev=1740047&view=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&r1=1740046&r2=1740047&view=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&r1=1740046&r2=1740047&view=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/MessageDispatch15Inte