[jira] [Comment Edited] (CASSANDRA-17243) Fix BYTES_PER_MEGABIT in StreamManager

2022-01-13 Thread Ekaterina Dimitrova (Jira)


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

Ekaterina Dimitrova edited comment on CASSANDRA-17243 at 1/13/22, 8:57 PM:
---

I am all in to fix correctness issues, I just wanted to be sure we consider the 
changed behavior. As you two have spent way more time than me with Cassandra in 
production I will trust your judgement in regards to which versions should be 
fixed. I definitely agree we have to update NEWS.txt for users' awareness. 
Thank you!


was (Author: e.dimitrova):
I am all in to fix correctness issues, I just wanted to be sure we consider the 
changed behavior. As you two have spent way more time than me with Cassandra in 
production I will trust your judgement. I definitely agree we have to update 
NEWS.txt for users' awareness. Thank you!

> Fix BYTES_PER_MEGABIT in StreamManager
> --
>
> Key: CASSANDRA-17243
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17243
> Project: Cassandra
>  Issue Type: Bug
>  Components: Consistency/Streaming
>Reporter: Ekaterina Dimitrova
>Assignee: Andres de la Peña
>Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
>
> While working on CASSANDRA-15234 I noticed BYTES_PER_MEGABIT constant in the 
> {code:java}
> StreamManager
> {code}
>  class. It was introduced in CASSANDRA-16959.
> The current formula converts actually bytes to mebibits. 
> The change needed for 3.0, 3.11 and 4.0(I am currently changing rate 
> parameters to be in MiB/s for trunk as part of CASSANDRA-15234):
> {code:java}
> public static final double BYTES_PER_MEGABIT = (1000 * 1000) / 8; // from bits
> {code}
> CC [~adelapena]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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



[jira] [Comment Edited] (CASSANDRA-17243) Fix BYTES_PER_MEGABIT in StreamManager

2022-01-12 Thread Ekaterina Dimitrova (Jira)


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

Ekaterina Dimitrova edited comment on CASSANDRA-17243 at 1/12/22, 1:54 PM:
---

Oh, indeed you are right it dates earlier than CASSANDRA-16959. 

Thinking more about it I am wondering whether it won't change a bit behavior 
then and which branches to target for the fix. 

Also, pre-4.0 we don't have _SetGetStreamThroughputTest_ which caught for me a 
precision issue with CASSANDRA-15234 that _StreamManagerTest_ missed. But I 
don't think this will be an issue here as 1000 * 1000 / 8 gives an integer 
result. More of a concern to me is the change of the rate limit.  On the other 
hand it is a correctness issue...

I want to think of it a bit more on my end.


was (Author: e.dimitrova):
Oh, indeed you are right it dates earlier than CASSANDRA-16959. 

Thinking more about it I am wondering whether it won't change a bit behavior 
then and which branches to target for the fix. 

Also, pre-4.0 we don't have _SetGetStreamThroughputTest_ which caught for me a 
precision issue with CASSANDRA-15234 that _StreamManagerTest_ missed. But I 
don't think this will be an issue here as 1000 * 1000 / 8 gives an integer 
result. More of a concern to me is the change of the rate limit.  

I want to think of it a bit more on my end.

> Fix BYTES_PER_MEGABIT in StreamManager
> --
>
> Key: CASSANDRA-17243
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17243
> Project: Cassandra
>  Issue Type: Bug
>  Components: Consistency/Streaming
>Reporter: Ekaterina Dimitrova
>Assignee: Andres de la Peña
>Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
>
> While working on CASSANDRA-15234 I noticed BYTES_PER_MEGABIT constant in the 
> {code:java}
> StreamManager
> {code}
>  class. It was introduced in CASSANDRA-16959.
> The current formula converts actually bytes to mebibits. 
> The change needed for 3.0, 3.11 and 4.0(I am currently changing rate 
> parameters to be in MiB/s for trunk as part of CASSANDRA-15234):
> {code:java}
> public static final double BYTES_PER_MEGABIT = (1000 * 1000) / 8; // from bits
> {code}
> CC [~adelapena]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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



[jira] [Comment Edited] (CASSANDRA-17243) Fix BYTES_PER_MEGABIT in StreamManager

2022-01-12 Thread Ekaterina Dimitrova (Jira)


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

Ekaterina Dimitrova edited comment on CASSANDRA-17243 at 1/12/22, 1:53 PM:
---

Oh, indeed you are right it dates earlier than CASSANDRA-16959. 

Thinking more about it I am wondering whether it won't change a bit behavior 
then and which branches to target for the fix. 

Also, pre-4.0 we don't have _SetGetStreamThroughputTest_ which caught for me a 
precision issue with CASSANDRA-15234 that _StreamManagerTest_ missed. But I 
don't think this will be an issue here as 1000 * 1000 / 8 gives an integer 
result. More of a concern to me is the change of the rate limit.  

I want to think of it a bit more on my end.


was (Author: e.dimitrova):
Oh, indeed you are right it dates earlier than CASSANDRA-16959. 

Thinking more about it I am wondering whether it won't change a bit behavior 
then and which branches to target for the fix. 

Pre-4.0 we don't have _SetGetStreamThroughputTest_ which caught for me a 
precision issue with CASSANDRA-15234 that _StreamManagerTest_ missed.

I want to think of it a bit more on my end.

> Fix BYTES_PER_MEGABIT in StreamManager
> --
>
> Key: CASSANDRA-17243
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17243
> Project: Cassandra
>  Issue Type: Bug
>  Components: Consistency/Streaming
>Reporter: Ekaterina Dimitrova
>Assignee: Andres de la Peña
>Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
>
> While working on CASSANDRA-15234 I noticed BYTES_PER_MEGABIT constant in the 
> {code:java}
> StreamManager
> {code}
>  class. It was introduced in CASSANDRA-16959.
> The current formula converts actually bytes to mebibits. 
> The change needed for 3.0, 3.11 and 4.0(I am currently changing rate 
> parameters to be in MiB/s for trunk as part of CASSANDRA-15234):
> {code:java}
> public static final double BYTES_PER_MEGABIT = (1000 * 1000) / 8; // from bits
> {code}
> CC [~adelapena]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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



[jira] [Comment Edited] (CASSANDRA-17243) Fix BYTES_PER_MEGABIT in StreamManager

2022-01-12 Thread Jira


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

Andres de la Peña edited comment on CASSANDRA-17243 at 1/12/22, 1:37 PM:
-

Very good catch. Indeed the mistake of confusing megabits with mebibits was 
originally introduced by CASSANDRA-5286, back in 2.0, and it has been 
overlooked by multiple fixes around that calculation.

The straightforward fix is simply the one mentioned by [~e.dimitrova]:
||PR||CI||
|[3.0|https://github.com/apache/cassandra/pull/1399]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1236/workflows/e3c3a592-4be7-4bab-8ced-ec6719e78a18]|
|[3.11|https://github.com/apache/cassandra/pull/1400]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1235/workflows/679af579-50d6-46b5-a5cb-9bf4b3986cc6]|
|[4.0|https://github.com/apache/cassandra/pull/1401]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/c8981fa3-5770-4970-beee-1bc44f3b04b8]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/27fbf649-60d5-4d8f-b6bf-0a0055fe8917]|
|[trunk|https://github.com/apache/cassandra/pull/1402]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/0805970f-2358-40f9-87c7-ac429c979c08]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/91b1bec3-8bac-4771-9597-64546d7a6100]|

The CI runs above contain some repeated runs of {{{}StreamManagerTest{}}}. Any 
test is going to be based on the definition of a megabit, so I don't know if we 
can add a new test for this.
 


was (Author: adelapena):
Very good catch. Indeed the mistake of confusing megabits with mebibits was 
originally introduced by CASSANDRA-5286, back in 2.0, and it has been 
overlooked by multiple fixes around that calculation.

The straightforward fix is simply the one mentioned by [~e.dimitrova]:
||PR||CI||
|[3.0|https://github.com/apache/cassandra/pull/1399]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1236/workflows/e3c3a592-4be7-4bab-8ced-ec6719e78a18]|
|[3.11|https://github.com/apache/cassandra/pull/1400]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1235/workflows/679af579-50d6-46b5-a5cb-9bf4b3986cc6]|
|[4.0|https://github.com/apache/cassandra/pull/1401]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/c8981fa3-5770-4970-beee-1bc44f3b04b8]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/27fbf649-60d5-4d8f-b6bf-0a0055fe8917]|
|[trunk|https://github.com/apache/cassandra/pull/1402]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/0805970f-2358-40f9-87c7-ac429c979c08]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/91b1bec3-8bac-4771-9597-64546d7a6100]|

The CI runs above contain some repeated runs of {{{}StreamManagerTest{}}}. Any 
test is going to be based on the definition of a megabit, so I don't know if we 
should add anything there.
 

> Fix BYTES_PER_MEGABIT in StreamManager
> --
>
> Key: CASSANDRA-17243
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17243
> Project: Cassandra
>  Issue Type: Bug
>  Components: Consistency/Streaming
>Reporter: Ekaterina Dimitrova
>Assignee: Andres de la Peña
>Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
>
> While working on CASSANDRA-15234 I noticed BYTES_PER_MEGABIT constant in the 
> {code:java}
> StreamManager
> {code}
>  class. It was introduced in CASSANDRA-16959.
> The current formula converts actually bytes to mebibits. 
> The change needed for 3.0, 3.11 and 4.0(I am currently changing rate 
> parameters to be in MiB/s for trunk as part of CASSANDRA-15234):
> {code:java}
> public static final double BYTES_PER_MEGABIT = (1000 * 1000) / 8; // from bits
> {code}
> CC [~adelapena]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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



[jira] [Comment Edited] (CASSANDRA-17243) Fix BYTES_PER_MEGABIT in StreamManager

2022-01-12 Thread Jira


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

Andres de la Peña edited comment on CASSANDRA-17243 at 1/12/22, 1:33 PM:
-

Very good catch. Indeed the mistake of confusing megabits with mebibits was 
originally introduced by CASSANDRA-5286, back in 2.0, and it has been 
overlooked by multiple fixes around that calculation.

The straightforward fix is simply the one mentioned by [~e.dimitrova]:
||PR||CI||
|[3.0|https://github.com/apache/cassandra/pull/1399]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1236/workflows/e3c3a592-4be7-4bab-8ced-ec6719e78a18]|
|[3.11|https://github.com/apache/cassandra/pull/1400]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1235/workflows/679af579-50d6-46b5-a5cb-9bf4b3986cc6]|
|[4.0|https://github.com/apache/cassandra/pull/1401]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/c8981fa3-5770-4970-beee-1bc44f3b04b8]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/27fbf649-60d5-4d8f-b6bf-0a0055fe8917]|
|[trunk|https://github.com/apache/cassandra/pull/1402]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/0805970f-2358-40f9-87c7-ac429c979c08]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/91b1bec3-8bac-4771-9597-64546d7a6100]|

The CI runs above contain some repeated runs of {{{}StreamManagerTest{}}}. Any 
test is going to be based on the definition of a megabit, so I don't know if we 
should add anything there.
 


was (Author: adelapena):
Very good catch. Indeed the mistake of confusing megabits with mebibits was 
originally introduced by CASSANDRA-5286, back in 2.0, and it has been 
overlooked by multiple fixes around that calculation.

The straightforward fix is simply the one mentioned by [~e.dimitrova]:
||PR||CI||
|[3.0|https://github.com/apache/cassandra/pull/1399]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1236/workflows/e3c3a592-4be7-4bab-8ced-ec6719e78a18]|
|[3.11|https://github.com/apache/cassandra/pull/1400]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1235/workflows/679af579-50d6-46b5-a5cb-9bf4b3986cc6]|
|[4.0|https://github.com/apache/cassandra/pull/1401]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/c8981fa3-5770-4970-beee-1bc44f3b04b8]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1238/workflows/27fbf649-60d5-4d8f-b6bf-0a0055fe8917]|
|[trunk|https://github.com/apache/cassandra/pull/1402]|[j8|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/0805970f-2358-40f9-87c7-ac429c979c08]
 
[j11|https://app.circleci.com/pipelines/github/adelapena/cassandra/1237/workflows/91b1bec3-8bac-4771-9597-64546d7a6100]|

The CI runs above contain some repeated runs of {{{}StreamManagerTest{}}}.
 

> Fix BYTES_PER_MEGABIT in StreamManager
> --
>
> Key: CASSANDRA-17243
> URL: https://issues.apache.org/jira/browse/CASSANDRA-17243
> Project: Cassandra
>  Issue Type: Bug
>  Components: Consistency/Streaming
>Reporter: Ekaterina Dimitrova
>Assignee: Andres de la Peña
>Priority: Normal
> Fix For: 3.0.x, 3.11.x, 4.0.x, 4.x
>
>
> While working on CASSANDRA-15234 I noticed BYTES_PER_MEGABIT constant in the 
> {code:java}
> StreamManager
> {code}
>  class. It was introduced in CASSANDRA-16959.
> The current formula converts actually bytes to mebibits. 
> The change needed for 3.0, 3.11 and 4.0(I am currently changing rate 
> parameters to be in MiB/s for trunk as part of CASSANDRA-15234):
> {code:java}
> public static final double BYTES_PER_MEGABIT = (1000 * 1000) / 8; // from bits
> {code}
> CC [~adelapena]



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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