[
https://issues.apache.org/jira/browse/IGNITE-18219?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Denis Chudov updated IGNITE-18219:
----------------------------------
Description:
*Motivation:*
Following code may be erroneous:
{code:java}
if (causalityToken <= actualToken0) {
return getValueForPreviousToken(causalityToken);
}
trimHistoryLock.readLock().lock();
try {
if (causalityToken <= actualToken0) {
return getValueForPreviousToken(causalityToken);
}
{code}
The original intention of making {{trimHistoryLock}} in {{VersionedValue}}
class was to make adding tokens to {{history}} and switching actual token
atomic. Seems that this atomicity has been lost after some changes. Need to
dive deeper into it and fix possible problems related to consistency of
internal mutable state of VersionedValue, and necessity of trimHistoryLock in
presence of {{updateMutex}}.
Current use cases of VersionedValue (VV) imply that VV actual version is
switched in single thread of configuration notifcator. On the other side,
{{#update}} method can be called concurrently from different threads, which
requires {{updateMutex}}, and switching actual version also should happen under
this mutex, as it clears the update future. {{trimHistoryLock}}, which is
read-write lock, is introduced due to {{history}} trimming, which can be done
under write lock, and all the other access to the {history}} can be done under
read lock. Possibly history trimming can be done under update mutex, and
trimHistoryLock would be not needed.
Also, there is possible bug with trimming history as the last value in hostory
can be not the same as actual token, we cant do direct arithmetics with token
computation, we should compare history size and maximum history size instead.
*Definition of done:*
There should be either one mutex left or necessity of both of them explained in
javadoc. Possibly, it is worth making multithreaded test for VersionedValue
which would change the history concurrently with high intensity.
was:
*Motivation:*
Following code may be erroneous:
{code:java}
if (causalityToken <= actualToken0) {
return getValueForPreviousToken(causalityToken);
}
trimHistoryLock.readLock().lock();
try {
if (causalityToken <= actualToken0) {
return getValueForPreviousToken(causalityToken);
}
{code}
The original intention of making {{trimHistoryLock}} in {{VersionedValue}}
class was to make adding tokens to {{history}} and switching actual token
atomic. Seems that this atomicity has been lost after some changes. Need to
dive deeper into it and fix possible problems related to consistency of
internal mutable state of VersionedValue, and necessity of trimHistoryLock in
presence of {{updateMutex}}.
*Definition of done:*
There should be either one mutex left or necessity of both of them explained in
javadoc. Possibly, it is worth making multithreaded test for VersionedValue
which would change the history concurrently with high intensity.
> Actual token is compared twice in VersionedValue#getInternal
> ------------------------------------------------------------
>
> Key: IGNITE-18219
> URL: https://issues.apache.org/jira/browse/IGNITE-18219
> Project: Ignite
> Issue Type: Bug
> Reporter: Denis Chudov
> Priority: Major
> Labels: ignite-3
>
> *Motivation:*
> Following code may be erroneous:
> {code:java}
> if (causalityToken <= actualToken0) {
> return getValueForPreviousToken(causalityToken);
> }
> trimHistoryLock.readLock().lock();
> try {
> if (causalityToken <= actualToken0) {
> return getValueForPreviousToken(causalityToken);
> }
> {code}
> The original intention of making {{trimHistoryLock}} in {{VersionedValue}}
> class was to make adding tokens to {{history}} and switching actual token
> atomic. Seems that this atomicity has been lost after some changes. Need to
> dive deeper into it and fix possible problems related to consistency of
> internal mutable state of VersionedValue, and necessity of trimHistoryLock in
> presence of {{updateMutex}}.
> Current use cases of VersionedValue (VV) imply that VV actual version is
> switched in single thread of configuration notifcator. On the other side,
> {{#update}} method can be called concurrently from different threads, which
> requires {{updateMutex}}, and switching actual version also should happen
> under this mutex, as it clears the update future. {{trimHistoryLock}}, which
> is read-write lock, is introduced due to {{history}} trimming, which can be
> done under write lock, and all the other access to the {history}} can be done
> under read lock. Possibly history trimming can be done under update mutex,
> and trimHistoryLock would be not needed.
> Also, there is possible bug with trimming history as the last value in
> hostory can be not the same as actual token, we cant do direct arithmetics
> with token computation, we should compare history size and maximum history
> size instead.
> *Definition of done:*
> There should be either one mutex left or necessity of both of them explained
> in javadoc. Possibly, it is worth making multithreaded test for
> VersionedValue which would change the history concurrently with high
> intensity.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
