You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cassandra.apache.org by Natnael Adere <na...@apple.com> on 2022/12/14 18:39:51 UTC
Issue when creating a stream session
Hello,
I am working on CASSANDRA-17199 <https://issues.apache.org/jira/browse/CASSANDRA-17199> and testing for this ticket has uncovered some issues with streaming. When creating a StreamSession we have an intent for to listen but that never happens. My concern is wether or not we should we listen and make sure the interface it implements is not dead code or delete the interface and all of its methods. Our style guide requires no dead code so this might be intentional, but when testing we see that StreamSession not listening. If it is intentional then I suppose we make sure that we are listening. Any opinions on what to do in this scenario?
(PROBLEM) The project compiles in both scenarios:
public class StreamSession implements IEndpointStateChangeSubscriber
public class StreamSession //implements IEndpointStateChangeSubscriber
Thanks,
Natnael
Re: Issue when creating a stream session
Posted by Aleksey Yeshchenko <al...@apple.com>.
Indeed, it is safe to clean up now.
Normally we prefer to do cleanups like that on trunk only, but I’d say feel free to remove this dead code in 4.0+ if that’s the branch you are targeting in CASSANDRA-17199.
> On 19 Dec 2022, at 03:53, Yuki Morishita <mo...@gmail.com> wrote:
>
> I dug the git history and it looks like CASSANDRA-3569 <https://issues.apache.org/jira/browse/CASSANDRA-3569> changed to not register to Gossip.
> https://github.com/apache/cassandra/commit/0f2d7d0b9540efa3ea3dfe4f8270c3635afdc63c
> (It removed the `register` part, not the `unregister` part though.)
>
> Since StreamSession is no longer registered to nor unregistered from Gosipper, I say the current "implements" code
> is a dead code and we can remove it safely.
>
>
> On Sat, Dec 17, 2022 at 4:57 AM David Capwell <dcapwell@apple.com <ma...@apple.com>> wrote:
>> This sounds like a bug to me, but would be good to get feedback from others who have touched Streaming…
>>
>> Repair will fail if membership notifies that a participate node was removed, so I think it makes sense for Streaming to also follow this behavior.
>>
>>> On Dec 14, 2022, at 1:22 PM, Natnael Adere <natnael_adere@apple.com <ma...@apple.com>> wrote:
>>>
>>> To give more context, StreamSession is not listening to Gossip for membership changes. Although it implements an interface for listening to membership changes, we do not register with Gossip and, therefore, never get these changes. This results in the IEndpointStateChangeSubscriber interface being dead code. My question is wether or not this is a bug to be fixed or code to delete. Currently, streaming does not fail on membership changes because of this problem. If Gossip says that a node was removed or restarted, should we fail the stream or not?
>>>
>>> Thanks,
>>> Natnael
>>>> On Dec 14, 2022, at 1:39 PM, Natnael Adere <natnael_adere@apple.com <ma...@apple.com>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> I am working on CASSANDRA-17199 <https://issues.apache.org/jira/browse/CASSANDRA-17199> and testing for this ticket has uncovered some issues with streaming. When creating a StreamSession we have an intent for to listen but that never happens. My concern is wether or not we should we listen and make sure the interface it implements is not dead code or delete the interface and all of its methods. Our style guide requires no dead code so this might be intentional, but when testing we see that StreamSession not listening. If it is intentional then I suppose we make sure that we are listening. Any opinions on what to do in this scenario?
>>>>
>>>>
>>>> (PROBLEM) The project compiles in both scenarios:
>>>> public class StreamSession implements IEndpointStateChangeSubscriber
>>>>
>>>> public class StreamSession //implements IEndpointStateChangeSubscriber
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Natnael
>>>
>>
Re: Issue when creating a stream session
Posted by Yuki Morishita <mo...@gmail.com>.
I dug the git history and it looks like CASSANDRA-3569
<https://issues.apache.org/jira/browse/CASSANDRA-3569> changed to not
register to Gossip.
https://github.com/apache/cassandra/commit/0f2d7d0b9540efa3ea3dfe4f8270c3635afdc63c
(It removed the `register` part, not the `unregister` part though.)
Since StreamSession is no longer registered to nor unregistered from
Gosipper, I say the current "implements" code
is a dead code and we can remove it safely.
On Sat, Dec 17, 2022 at 4:57 AM David Capwell <dc...@apple.com> wrote:
> This sounds like a bug to me, but would be good to get feedback from
> others who have touched Streaming…
>
> Repair will fail if membership notifies that a participate node was
> removed, so I think it makes sense for Streaming to also follow this
> behavior.
>
> On Dec 14, 2022, at 1:22 PM, Natnael Adere <na...@apple.com>
> wrote:
>
> To give more context, StreamSession is not listening to Gossip for
> membership changes. Although it implements an interface for listening to
> membership changes, we do not register with Gossip and, therefore, never
> get these changes. This results in the IEndpointStateChangeSubscriber
> interface being dead code. My question is wether or not this is a bug to be
> fixed or code to delete. Currently, streaming does not fail on membership
> changes because of this problem. If Gossip says that a node was removed or
> restarted, should we fail the stream or not?
>
> Thanks,
> Natnael
>
> On Dec 14, 2022, at 1:39 PM, Natnael Adere <na...@apple.com>
> wrote:
>
> Hello,
>
> I am working on CASSANDRA-17199
> <https://issues.apache.org/jira/browse/CASSANDRA-17199> and testing for
> this ticket has uncovered some issues with streaming. When creating a
> StreamSession we have an intent for to listen but that never happens. My
> concern is wether or not we should we listen and make sure the interface
> it implements is not dead code or delete the interface and all of its
> methods. Our style guide requires no dead code so this might be
> intentional, but when testing we see that StreamSession not listening. If
> it is intentional then I suppose we make sure that we are listening. Any
> opinions on what to do in this scenario?
>
>
> (PROBLEM) The project compiles in both scenarios:
>
> public class StreamSession implements IEndpointStateChangeSubscriber
>
>
> public class StreamSession //implements IEndpointStateChangeSubscriber
>
>
>
> Thanks,
>
> Natnael
>
>
>
>
Re: Issue when creating a stream session
Posted by David Capwell <dc...@apple.com>.
This sounds like a bug to me, but would be good to get feedback from others who have touched Streaming…
Repair will fail if membership notifies that a participate node was removed, so I think it makes sense for Streaming to also follow this behavior.
> On Dec 14, 2022, at 1:22 PM, Natnael Adere <na...@apple.com> wrote:
>
> To give more context, StreamSession is not listening to Gossip for membership changes. Although it implements an interface for listening to membership changes, we do not register with Gossip and, therefore, never get these changes. This results in the IEndpointStateChangeSubscriber interface being dead code. My question is wether or not this is a bug to be fixed or code to delete. Currently, streaming does not fail on membership changes because of this problem. If Gossip says that a node was removed or restarted, should we fail the stream or not?
>
> Thanks,
> Natnael
>> On Dec 14, 2022, at 1:39 PM, Natnael Adere <natnael_adere@apple.com <ma...@apple.com>> wrote:
>>
>> Hello,
>>
>> I am working on CASSANDRA-17199 <https://issues.apache.org/jira/browse/CASSANDRA-17199> and testing for this ticket has uncovered some issues with streaming. When creating a StreamSession we have an intent for to listen but that never happens. My concern is wether or not we should we listen and make sure the interface it implements is not dead code or delete the interface and all of its methods. Our style guide requires no dead code so this might be intentional, but when testing we see that StreamSession not listening. If it is intentional then I suppose we make sure that we are listening. Any opinions on what to do in this scenario?
>>
>>
>> (PROBLEM) The project compiles in both scenarios:
>> public class StreamSession implements IEndpointStateChangeSubscriber
>>
>> public class StreamSession //implements IEndpointStateChangeSubscriber
>>
>>
>> Thanks,
>>
>> Natnael
>
Re: Issue when creating a stream session
Posted by Natnael Adere <na...@apple.com>.
To give more context, StreamSession is not listening to Gossip for membership changes. Although it implements an interface for listening to membership changes, we do not register with Gossip and, therefore, never get these changes. This results in the IEndpointStateChangeSubscriber interface being dead code. My question is wether or not this is a bug to be fixed or code to delete. Currently, streaming does not fail on membership changes because of this problem. If Gossip says that a node was removed or restarted, should we fail the stream or not?
Thanks,
Natnael
> On Dec 14, 2022, at 1:39 PM, Natnael Adere <na...@apple.com> wrote:
>
> Hello,
>
> I am working on CASSANDRA-17199 <https://issues.apache.org/jira/browse/CASSANDRA-17199> and testing for this ticket has uncovered some issues with streaming. When creating a StreamSession we have an intent for to listen but that never happens. My concern is wether or not we should we listen and make sure the interface it implements is not dead code or delete the interface and all of its methods. Our style guide requires no dead code so this might be intentional, but when testing we see that StreamSession not listening. If it is intentional then I suppose we make sure that we are listening. Any opinions on what to do in this scenario?
>
>
> (PROBLEM) The project compiles in both scenarios:
> public class StreamSession implements IEndpointStateChangeSubscriber
>
> public class StreamSession //implements IEndpointStateChangeSubscriber
>
>
> Thanks,
>
> Natnael