[jira] [Commented] (CASSANDRA-14754) Add verification of state machine in StreamSession

2020-03-27 Thread Sergio Bossa (Jira) 


[ 
https://issues.apache.org/jira/browse/CASSANDRA-14754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17068579#comment-17068579
 ] 

Sergio Bossa commented on CASSANDRA-14754:
--

Thanks [~jasonstack]. Also see CASSANDRA-15667.

> Add verification of state machine in StreamSession
> --
>
> Key: CASSANDRA-14754
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14754
> Project: Cassandra
>  Issue Type: Task
>  Components: Legacy/Streaming and Messaging
>Reporter: Jason Brown
>Assignee: ZhaoYang
>Priority: Normal
> Fix For: 4.x
>
>
> {{StreamSession}} contains an implicit state machine, but we have no 
> verification of the safety of the transitions between states. For example, we 
> have no checks to ensure we cannot leave the final states (COMPLETED, FAILED).
> I propose we add some program logic in {{StreamSession}}, tests, and 
> documentation to ensure the correctness of the state transitions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

[jira] [Commented] (CASSANDRA-14754) Add verification of state machine in StreamSession

2020-03-26 Thread ZhaoYang (Jira) 


[ 
https://issues.apache.org/jira/browse/CASSANDRA-14754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17068273#comment-17068273
 ] 

ZhaoYang commented on CASSANDRA-14754:
--

bq.  for example, moving from INITIALIZED to COMPLETE is valid, but only if 
there's nothing to actually stream, which is not expressed in the current 
validation (IIUIC, correct me if I'm wrong). In other words, I think it would 
be best to implement a proper state machine, but is it really necessary for 4.0?

Indeed, there isn't much value on validating the state transition alone without 
checking other prerequisites.  Moved to 4.x

I will port some simplifications from this patch into CASSANDRA-15665, 
CASSANDRA-15666.

> Add verification of state machine in StreamSession
> --
>
> Key: CASSANDRA-14754
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14754
> Project: Cassandra
>  Issue Type: Task
>  Components: Legacy/Streaming and Messaging
>Reporter: Jason Brown
>Assignee: ZhaoYang
>Priority: Normal
> Fix For: 4.x
>
>
> {{StreamSession}} contains an implicit state machine, but we have no 
> verification of the safety of the transitions between states. For example, we 
> have no checks to ensure we cannot leave the final states (COMPLETED, FAILED).
> I propose we add some program logic in {{StreamSession}}, tests, and 
> documentation to ensure the correctness of the state transitions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

[jira] [Commented] (CASSANDRA-14754) Add verification of state machine in StreamSession

2020-03-26 Thread Sergio Bossa (Jira) 


[ 
https://issues.apache.org/jira/browse/CASSANDRA-14754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17067826#comment-17067826
 ] 

Sergio Bossa commented on CASSANDRA-14754:
--

I think just validating state transitions doesn't really add much: for example, 
moving from {{INITIALIZED}} to {{COMPLETE}} is 
[valid|https://github.com/apache/cassandra/pull/480/files#diff-b397cc5438b1a4be20f026c9ec9ecd6eR215],
 but only if there's nothing to actually stream, which is not expressed in the 
current validation (IIUIC, correct me if I'm wrong). In other words, I think it 
would be best to implement a proper state machine, but is it really necessary 
for 4.0? OTOH, I think we should focus on other {{StreamSession}} bugs such as 
CASSANDRA-15665 and CASSANDRA-15666. Thoughts?

> Add verification of state machine in StreamSession
> --
>
> Key: CASSANDRA-14754
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14754
> Project: Cassandra
>  Issue Type: Task
>  Components: Legacy/Streaming and Messaging
>Reporter: Jason Brown
>Assignee: ZhaoYang
>Priority: Normal
> Fix For: 4.0
>
>
> {{StreamSession}} contains an implicit state machine, but we have no 
> verification of the safety of the transitions between states. For example, we 
> have no checks to ensure we cannot leave the final states (COMPLETED, FAILED).
> I propose we add some program logic in {{StreamSession}}, tests, and 
> documentation to ensure the correctness of the state transitions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

[jira] [Commented] (CASSANDRA-14754) Add verification of state machine in StreamSession

2020-03-26 Thread ZhaoYang (Jira) 


[ 
https://issues.apache.org/jira/browse/CASSANDRA-14754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17067501#comment-17067501
 ] 

ZhaoYang commented on CASSANDRA-14754:
--

Thanks for the feedback.. Update the patch with the flow diagram.

> Add verification of state machine in StreamSession
> --
>
> Key: CASSANDRA-14754
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14754
> Project: Cassandra
>  Issue Type: Task
>  Components: Legacy/Streaming and Messaging
>Reporter: Jason Brown
>Assignee: ZhaoYang
>Priority: Normal
> Fix For: 4.0
>
>
> {{StreamSession}} contains an implicit state machine, but we have no 
> verification of the safety of the transitions between states. For example, we 
> have no checks to ensure we cannot leave the final states (COMPLETED, FAILED).
> I propose we add some program logic in {{StreamSession}}, tests, and 
> documentation to ensure the correctness of the state transitions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

[jira] [Commented] (CASSANDRA-14754) Add verification of state machine in StreamSession

2020-03-25 Thread Benjamin Lerer (Jira) 


[ 
https://issues.apache.org/jira/browse/CASSANDRA-14754?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17066591#comment-17066591
 ] 

Benjamin Lerer commented on CASSANDRA-14754:


My understanding of the possible states transition is the following:
{code}
 
  +-+--> FAILED <-+
  | |  ^  |  
  | |  |  |
INITIALIZED --> PREPARING --> STREAMING --> WAIT_COMPLETE ---> COMPLETED
  | | ^^
  | | if preview  ||
  | +-+|
  |   nothing to request or to transfer|
  ++ 
  nothing to request or to transfer 

{code}

It should not be possible for example to go from {{INITIALIZED}} to 
{{STREAMING}}.
The current patch does not check that type of state transitions and it might be 
good if it does.

Otherwise, the patch is firing an `IllegalStateException` in case of invalid 
state transition. It seems fine to me but I am not an expert in this part of 
the code.
[~jasobrown], [~djoshi] do you have any concerns with the approach?

> Add verification of state machine in StreamSession
> --
>
> Key: CASSANDRA-14754
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14754
> Project: Cassandra
>  Issue Type: Task
>  Components: Legacy/Streaming and Messaging
>Reporter: Jason Brown
>Assignee: ZhaoYang
>Priority: Normal
> Fix For: 4.0
>
>
> {{StreamSession}} contains an implicit state machine, but we have no 
> verification of the safety of the transitions between states. For example, we 
> have no checks to ensure we cannot leave the final states (COMPLETED, FAILED).
> I propose we add some program logic in {{StreamSession}}, tests, and 
> documentation to ensure the correctness of the state transitions.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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