[
https://issues.apache.org/jira/browse/CASSANDRA-16681?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17544326#comment-17544326
]
Piotr Kolaczkowski edited comment on CASSANDRA-16681 at 5/31/22 11:25 AM:
--------------------------------------------------------------------------
> I was proposing removing the count entirely, as it is a minor optimisation.
> Simply use the three fields and their null/non-null status.
I agree, this makes sense as well. So that's actually not that complex as I
initially thought. The fact we have a fixed number of items (3) and if we
remove the counter, we could allow "add from one thread, remove from any" and
probably even volatile would be enough.
Well, now I actually like this idea, and you seem to have (almost) convinced
me. :)
There is also some nice symmetry with chunks, which can also only be allocated
from by a single thread, yet any thread can return buffers to them.
One minor concern is though, that what about eviction racing with recycle. I
mean, the owner evicts a chunk and at the same time the application returns the
last buffer into that chunk and triggers recycling (on another thread). I guess
one must be careful with first nulling the owner and then removing from the
queue to prevent this....
Anyway, that's slightly more complex than the isOwner param / thread check, so
let's leave it for the future.
> Some time in future, we should consider refactoring more fully, to perhaps
> distinguish the `LocalPool` and `TinyPool` concepts more fully so that we do
> not make this kind of mistake, as the behaviour is subtly different between
> the two, and it is easy to forget this and fail to account for the
> special-casing.
+1
was (Author: pkolaczk):
> I was proposing removing the count entirely, as it is a minor optimisation.
> Simply use the three fields and their null/non-null status.
I agree, this makes sense as well. So that's actually not that complex as I
initially thought. The fact we have a fixed number of items (3) and if we
remove the counter, we could allow "add from one thread, remove from any" and
probably even volatile would be enough (no CAS needed, because removers would
never race for the same item, as you can't recycle the same chunk more than
once).
Well, now I actually like this idea, and you seem to have convinced me. :)
There is also some nice symmetry with chunks, which can also only be allocated
from by a single thread, yet any thread can return buffers to them.
> Some time in future, we should consider refactoring more fully, to perhaps
> distinguish the `LocalPool` and `TinyPool` concepts more fully so that we do
> not make this kind of mistake, as the behaviour is subtly different between
> the two, and it is easy to forget this and fail to account for the
> special-casing.
+1
> org.apache.cassandra.utils.memory.LongBufferPoolTest - tests are flaky
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-16681
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16681
> Project: Cassandra
> Issue Type: Bug
> Components: CI
> Reporter: Ekaterina Dimitrova
> Assignee: Piotr Kolaczkowski
> Priority: Normal
> Fix For: 3.11.x, 4.0.x, 4.x
>
> Attachments: 0001-Fix-memoryInUse-counter-in-BufferPool.patch,
> 0002-Multiple-fixes-in-BufferPool-and-LongBufferPoolTest.patch
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Jenkins history:
> [https://jenkins-cm4.apache.org/job/Cassandra-4.0/50/testReport/junit/org.apache.cassandra.utils.memory/LongBufferPoolTest/testPoolAllocateWithRecyclePartially/history/]
> Fails being run in a loop in CircleCI:
> https://app.circleci.com/pipelines/github/ekaterinadimitrova2/cassandra/844/workflows/945011f4-00ac-4678-89f6-5c0db0a40169/jobs/5008
>
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
