[jira] [Comment Edited] (CASSANDRA-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17521404#comment-17521404
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 4/13/22 1:37 AM:
--
So I reran the upgrade tests with 3.11 in CircleCI without the patch. -
https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1533/workflows/118fff4b-1d4f-4fd6-8b2d-c33dd8f76bde/jobs/9933/tests
The only two tests that failed with my patch and I don't see in the list of
failures without are:
upgrade_tests.cql_tests.TestCQLNodes2RF1_Upgrade_indev_3_11_x_To_indev_trunk -
I found it in Butler, trunk
(https://ci-cassandra.apache.org/job/Cassandra-trunk/1076/testReport/dtest-upgrade.upgrade_tests.cql_tests/TestCQLNodes2RF1_Upgrade_indev_3_0_x_To_indev_trunk/test_noncomposite_static_cf/)
test_noncomposite_static_cf - found it again on trunk...
https://ci-cassandra.apache.org/job/Cassandra-trunk/1076/testReport/dtest-upgrade.upgrade_tests.cql_tests/cls/test_noncomposite_static_cf/
So to conclude, I see the same failures, except two which for some reason in
Butler appear under trunk and not 3.11... I am not sure why I see what I see...
I got +1 on the patch on green CI from [~blerer] in Slack.
I consider all failures known and I will open a ticket to align how we run the
upgrade tests in Circle and in Jenkins to ensure we don't miss anything...
I will wait until tomorrow to commit the patch as there is some Jenkins issue
and I see the last runs marked in red. Infra is checking.
was (Author: e.dimitrova):
So I reran the upgrade tests with 3.11 in CircleCI without the patch. -
https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1533/workflows/118fff4b-1d4f-4fd6-8b2d-c33dd8f76bde/jobs/9933/tests
The only two tests I don't see in the list of failures are:
upgrade_tests.cql_tests.TestCQLNodes2RF1_Upgrade_indev_3_11_x_To_indev_trunk -
I found it in Butler, trunk
(https://ci-cassandra.apache.org/job/Cassandra-trunk/1076/testReport/dtest-upgrade.upgrade_tests.cql_tests/TestCQLNodes2RF1_Upgrade_indev_3_0_x_To_indev_trunk/test_noncomposite_static_cf/)
test_noncomposite_static_cf - found it again on trunk...
https://ci-cassandra.apache.org/job/Cassandra-trunk/1076/testReport/dtest-upgrade.upgrade_tests.cql_tests/cls/test_noncomposite_static_cf/
So to conclude, I see the same failures, except two which for some reason in
Butler appear under trunk and not 3.11... I am not sure why I see what I see...
I got +1 on the patch on green CI from [~blerer] in Slack.
I consider all failures known and I will open a ticket to align how we run the
upgrade tests in Circle and in Jenkins to ensure we don't miss anything...
I will wait until tomorrow to commit the patch as there is some Jenkins issue
and I see the last runs marked in red. Infra is checking.
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 3.11.x, 4.0.x, 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
> Every time we return new bytebuffer object. Please do let me know If there
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0});
> // false
> }
> {code}
--
This message was sent by Atlassian Jira
[jira] [Comment Edited] (CASSANDRA-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17520837#comment-17520837
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 4/12/22 10:05 PM:
---
The upgrade_udtfix_test look suspicious from the perspective they don't fail in
Jenkins (talking about Cassandra-3.11).
I pushed in a loop with the patch:
[https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1532/workflows/dc444e1f-42ca-4993-b053-5a9c69ba1c2d]
Without the patch:
[https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1533/workflows/118fff4b-1d4f-4fd6-8b2d-c33dd8f76bde]
Everything is green and when you try to look at the logs - test runs were just
skipped this made me check Jenkins, it seems those tests are skipped also
there...
Maybe [~jlewandowski], [~brandon.williams] or [~mck] will know something? I see
the three of you were interacting with those tests before.
The rest of the failures are all known with associated tickets.
was (Author: e.dimitrova):
The upgrade_udtfix_test look suspicious from the perspective they don't fail in
Jenkins.
I pushed in a loop with the patch:
[https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1532/workflows/dc444e1f-42ca-4993-b053-5a9c69ba1c2d]
Without the patch:
[https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/1533/workflows/118fff4b-1d4f-4fd6-8b2d-c33dd8f76bde]
Everything is green and when you try to look at the logs - test runs were just
skipped this made me check Jenkins, it seems those tests are skipped also
there...
Maybe [~jlewandowski], [~brandon.williams] or [~mck] will know something? I see
the three of you were interacting with those tests before.
The rest of the failures are all known with associated tickets.
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 3.11.x, 4.0.x, 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
> Every time we return new bytebuffer object. Please do let me know If there
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0});
> // false
> }
> {code}
--
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-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17457212#comment-17457212
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 12/10/21, 3:35 PM:
Thank you both, actually Benjamin made a pass and there was also another issue
with the trunk patch that the tests didn't catch but I got side-tracked and
left to follow up on this when there is more time to work on it and not jumping
in between other tasks as it is important to do it right.
Quick suggestion - should we apply the 3.11 patch and fix the bug for our users
and open follow up ticket if you think it is really worth it to pursue
something more at this point? [~marcuse] , [~blerer] , [~benedict] , what do
you think about that?
was (Author: e.dimitrova):
Thank you both, actually Benjamin made a pass and there was also another issue
with the trunk that the tests didn't catch but I got side-tracked and left to
follow up on this when there is more time to work on it and not jumping in
between other tasks as it is important to do it right.
Quick suggestion - should we apply the 3.11 patch and fix the bug for our users
and open follow up ticket if you think it is really worth it to pursue
something more at this point? [~marcuse] , [~blerer] , [~benedict] , what do
you think about that?
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 3.11.x, 4.0.x, 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
> Every time we return new bytebuffer object. Please do let me know If there
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0});
> // false
> }
> {code}
--
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-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17457212#comment-17457212
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 12/10/21, 3:33 PM:
Thank you both, actually Benjamin made a pass and there was also another issue
with the trunk that the tests didn't catch but I got side-tracked and left to
follow up on this when there is more time to work on it and not jumping in
between other tasks as it is important to do it right.
Quick suggestion - should we apply the 3.11 patch and fix the bug for our users
and open follow up ticket if you think it is really worth it to pursue
something more at this point? [~marcuse] , [~blerer] , [~benedict] , what do
you think about that?
was (Author: e.dimitrova):
Thank you both, actually Benjamin made a pass and there was also another issue
that the tests didn't catch but I got side-tracked and left to follow up on
this when there is more time to work on it and not jumping in between other
tasks as it is important to do it right.
Quick suggestion - should we apply the 3.11 patch and fix the bug for our users
and open follow up ticket if you think it is really worth it to pursue
something more at this point? [~marcuse] , [~blerer] , [~benedict] , what do
you think about that?
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 3.11.x, 4.0.x, 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
> Every time we return new bytebuffer object. Please do let me know If there
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0});
> // false
> }
> {code}
--
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-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17425118#comment-17425118
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 10/6/21, 5:37 PM:
---
I reworked 3.11 and 4.0 and pushed additional changes for trunk based on
Stefania Alborghetti's patch. (I will add her as author at the end before
commit)
*trunk:* Byte buffers of {{BooleanSerializer}} are now read-only. We cannot
make them on-heap read-only, as we would need to change our code in several key
places which seems not worth it at this point. However, buffers can be off-heap
read-only. Note that this only offers partial protection against put calls done
on the buffer itself. It will not protect, amongst several cases, for put calls
where the read-only buffer is the source, as it was the case in
{{AbstractCompositeType}}. In this case, the position will still be advanced.
To make these buffers completely safe we would need to duplicate them and
accept the additional GC pressure.
||Patch||CI||
|[3.11|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-3.11?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1171/#showFailuresLink]|
|[4.0|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-4.0?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1172/#showFailuresLink]|
|[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-trunk?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1183/#showFailuresLink]|
I don't see any related issues in the CI runs.
[~blerer], [~ifesdjeen], as you are already familiar with this, does anyone of
you have time for review?
was (Author: e.dimitrova):
I reworked 3.11 and 4.0 and pushed another additional changes for trunk based
on Stefania Alborghetti's patch. (I will add her as author at the end before
commit)
*trunk:* Byte buffers of {{BooleanSerializer}} are now read-only. We cannot
make them on-heap read-only, as we would need to change our code in several key
places which seems not worth it at this point. However, buffers can be off-heap
read-only. Note that this only offers partial protection against put calls done
on the buffer itself. It will not protect, amongst several cases, for put calls
where the read-only buffer is the source, as it was the case in
{{AbstractCompositeType}}. In this case, the position will still be advanced.
To make these buffers completely safe we would need to duplicate them and
accept the additional GC pressure.
||Patch||CI||
|[3.11|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-3.11?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1171/#showFailuresLink]|
|[4.0|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-4.0?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1172/#showFailuresLink]|
|[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-trunk?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1183/#showFailuresLink]|
I don't see any related issues in the CI runs.
[~blerer], [~ifesdjeen], as you are already familiar with this, does anyone of
you have time for review?
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/ca
[jira] [Comment Edited] (CASSANDRA-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17425118#comment-17425118
]
Ekaterina Dimitrova edited comment on CASSANDRA-14752 at 10/6/21, 5:36 PM:
---
I reworked 3.11 and 4.0 and pushed another additional changes for trunk based
on Stefania Alborghetti's patch. (I will add her as author at the end before
commit)
*trunk:* Byte buffers of {{BooleanSerializer}} are now read-only. We cannot
make them on-heap read-only, as we would need to change our code in several key
places which seems not worth it at this point. However, buffers can be off-heap
read-only. Note that this only offers partial protection against put calls done
on the buffer itself. It will not protect, amongst several cases, for put calls
where the read-only buffer is the source, as it was the case in
{{AbstractCompositeType}}. In this case, the position will still be advanced.
To make these buffers completely safe we would need to duplicate them and
accept the additional GC pressure.
||Patch||CI||
|[3.11|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-3.11?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1171/#showFailuresLink]|
|[4.0|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-4.0?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1172/#showFailuresLink]|
|[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-trunk?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1183/#showFailuresLink]|
I don't see any related issues in the CI runs.
[~blerer], [~ifesdjeen], as you are already familiar with this, does anyone of
you have time for review?
was (Author: e.dimitrova):
I reworked 3.11 and 4.0 and pushed another additional changes for trunk based
on Stefania Alborghetti's patch. (I will add her as author at the end before
commit)
4.0: Byte buffers of {{BooleanSerializer}} are now read-only. We cannot make
them on-heap read-only, as we would need to change our code in several key
places which seems not worth it at this point. However, buffers can be off-heap
read-only. Note that this only offers partial protection against put calls done
on the buffer itself. It will not protect, amongst several cases, for put calls
where the read-only buffer is the source, as it was the case in
{{AbstractCompositeType}}. In this case, the position will still be advanced.
To make these buffers completely safe we would need to duplicate them and
accept the additional GC pressure.
||Patch||CI||
|[3.11|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-3.11?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1171/#showFailuresLink]|
|[4.0|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-4.0?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1172/#showFailuresLink]|
|[trunk|https://github.com/apache/cassandra/compare/trunk...ekaterinadimitrova2:14752-trunk?expand=1]|[Jenkins|https://ci-cassandra.apache.org/job/Cassandra-devbranch/1183/#showFailuresLink]|
I don't see any related issues in the CI runs.
[~blerer], [~ifesdjeen], as you are already familiar with this, does anyone of
you have time for review?
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Legacy/Core
>Reporter: Varun Barala
>Assignee: Ekaterina Dimitrova
>Priority: Normal
> Fix For: 4.x
>
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. -IMO Using
> static is not a good idea here.-
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apach
[jira] [Comment Edited] (CASSANDRA-14752) serializers/BooleanSerializer.java is using static bytebuffers which may cause problem for subsequent operations
[
https://issues.apache.org/jira/browse/CASSANDRA-14752?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16617276#comment-16617276
]
Varun Barala edited comment on CASSANDRA-14752 at 9/17/18 9:32 AM:
---
I found there are too many usages of `AbstractCompositeType#fromString()`.
One way to corrupt the data:-
Table schema:-
{code:java}
CREATE TABLE ks1.table1 (
t_id boolean,
id boolean,
ck boolean,
nk boolean,
PRIMARY KEY ((t_id,id),ck)
);{code}
Insert statement:-
{code:java}
insert into ks1.table1 (t_id, ck, id, nk)
VALUES (true, false, false, true);
{code}
Now run nodetool command to get the SSTable for given key:-
{code:java}
bin/nodetool getsstables ks1 table1 "false:true"
{code}
Basically, this operation will modify the positions.
Insert again:-
{code:java}
insert into ks1.table1 (t_id, ck, id, nk)
VALUES (true, true, false, true);
{code}
select data from this table:-
{code:java}
true,false,false,true
null,null,null,null
{code}
So now all boolean type data will be written as null.
was (Author: varuna):
I found there are too many usages of `AbstractCompositeType#fromString()`.
One way to corrupt the data:-
Table schema:-
{code:java}
CREATE TABLE ks1.table1 (
t_id boolean,
id boolean,
ck boolean,
nk boolean,
PRIMARY KEY ((t_id,id),ck)
);{code}
Insert statement:-
{code:java}
insert into ks1.table1 (tenant_id, ck, id, nk)
VALUES (true, false, false, true);
{code}
Now run nodetool command to get the SSTable for given key:-
{code:java}
bin/nodetool getsstables ks1 table1 "false:true"
{code}
Basically, this operation will modify the positions.
Insert again:-
{code:java}
insert into ks1.table1 (tenant_id, ck, id, nk)
VALUES (true, true, false, true);
{code}
select data from this table:-
{code:java}
true,false,false,true
null,null,null,null
{code}
So now all boolean type data will be written as null.
> serializers/BooleanSerializer.java is using static bytebuffers which may
> cause problem for subsequent operations
>
>
> Key: CASSANDRA-14752
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14752
> Project: Cassandra
> Issue Type: Bug
> Components: Core
>Reporter: Varun Barala
>Priority: Major
> Attachments: patch, patch-modified
>
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L26]
> It has two static Bytebuffer variables:-
> {code:java}
> private static final ByteBuffer TRUE = ByteBuffer.wrap(new byte[]{1});
> private static final ByteBuffer FALSE = ByteBuffer.wrap(new byte[]{0});{code}
> What will happen if the position of these Bytebuffers is being changed by
> some other operations? It'll affect other subsequent operations. IMO Using
> static is not a good idea here.
> A potential place where it can become problematic:
> [https://github.com/apache/cassandra/blob/cassandra-2.1.13/src/java/org/apache/cassandra/db/marshal/AbstractCompositeType.java#L243]
> Since we are calling *`.remaining()`* It may give wrong results _i.e 0_ if
> these Bytebuffers have been used previously.
> Solution:
>
> [https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/serializers/BooleanSerializer.java#L42]
> Every time we return new bytebuffer object. Please do let me know If there
> is a better way. I'd like to contribute. Thanks!!
> {code:java}
> public ByteBuffer serialize(Boolean value)
> {
> return (value == null) ? ByteBufferUtil.EMPTY_BYTE_BUFFER
> : value ? ByteBuffer.wrap(new byte[] {1}) : ByteBuffer.wrap(new byte[] {0});
> // false
> }
> {code}
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
