You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/01/18 09:09:26 UTC
[GitHub] [bookkeeper] Shoothzj opened a new issue #2544: ConcurrentModificationException when print LinkedList
Shoothzj opened a new issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544
**BUG REPORT**
***Describe the bug***

***To Reproduce***

### What should we do ?
Do not print the linkedList
or
use read write lock to protect the list
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762298777
> that variable should be a CopyOnWriteArrayList and not a simple LinkedList
>
> see here:
> https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java#L277
>
> probably this issue is a result of a refactor
>
> otherwise we should take care of synchronization in all of the accesses to the variable.
> given the hierarchy of classes I suggest to change the type to CopyOnWriteArrayList here
> https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java#L53
Apparently, change it to CopyOnWriteArrayList can fix this problem.
But may be there is another solution, see these code:


May be we should add synchronize on method **commitEntryMemTableFlush** and **prepareSortedLedgerStorageCheckpoint** in class EntryLogManagerForSingleEntryLog.
These code are committed in year 2018. I don't think it's a refactor probleam
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli edited a comment on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
eolivelli edited a comment on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762256303
that variable should be a CopyOnWriteArrayList and not a simple LinkedList
see here:
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java#L277
probably this issue is a result of a refactor
otherwise we should take care of synchronization in all of the accesses to the variable.
given the hierarchy of classes I suggest to change the type to CopyOnWriteArrayList
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762303349
probably add those new synchronization points may cause additional issues
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj edited a comment on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj edited a comment on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762188925
> LinkedList is not thread safe. Any Bookie error log or some other information? @Shoothzj
Currently I have not log related to it, it has been rotated. Maybe concurrently open the ledger and flush?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj edited a comment on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj edited a comment on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762298777
> that variable should be a CopyOnWriteArrayList and not a simple LinkedList
>
> see here:
> https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java#L277
>
> probably this issue is a result of a refactor
>
> otherwise we should take care of synchronization in all of the accesses to the variable.
> given the hierarchy of classes I suggest to change the type to CopyOnWriteArrayList here
> https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java#L53
Apparently, change it to CopyOnWriteArrayList can fix this problem.
But may be there is another solution, see these code:


May be we should add synchronize on method **commitEntryMemTableFlush** and **prepareSortedLedgerStorageCheckpoint** in class **EntryLogManagerForSingleEntryLog**.
These code are committed in year 2018. I don't think it's a refactor probleam
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762314932
> probably add those new synchronization points may cause additional issues
The two children class of EntryLogManagerBase use different trick of sync.
One **EntryLogManagerForEntryLogPerLedger** use CopyOnWriteArrayList,
Another class EntryLogManagerForSingleEntryLog use volatile for replace reference, and synchronize for add
If we change **EntryLogManagerForSingleEntryLog** to CopyOnWriteArrayList, the other synchronize block like

seems useless, can be replace as
```java
rotatedLogChannels.clear();
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762256303
that variable should be a CopyOnWriteArrayList and not a simple LinkedList
see here:
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java#L277
probably this issue is a result of a refactor
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj removed a comment on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj removed a comment on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762533327
> probably add those new synchronization points may cause additional issues
This annotation may prove that add sync is right, how about put sync on this method

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli edited a comment on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
eolivelli edited a comment on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762256303
that variable should be a CopyOnWriteArrayList and not a simple LinkedList
see here:
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java#L277
probably this issue is a result of a refactor
otherwise we should take care of synchronization in all of the accesses to the variable.
given the hierarchy of classes I suggest to change the type to CopyOnWriteArrayList here
https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java#L53
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] WJL3333 commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
WJL3333 commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762136148
LinkedList is not thread safe. Any Bookie error log or some other information? @Shoothzj
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762188925
> LinkedList is not thread safe. Any Bookie error log or some other information? @Shoothzj
Currently I have not log according to it. Maybe concurrently open the ledger and flush?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762533327
> probably add those new synchronization points may cause additional issues
This annotation may prove that add sync is right, how about put sync on this method

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Shoothzj commented on issue #2544: ConcurrentModificationException when print LinkedList
Posted by GitBox <gi...@apache.org>.
Shoothzj commented on issue #2544:
URL: https://github.com/apache/bookkeeper/issues/2544#issuecomment-762535378
> probably add those new synchronization points may cause additional issues
Refer to the addEntry method,
**EntryLogManagerForEntryLogPerLedger** locks per ledger
**EntryLogManagerForSingleEntryLog** locks itself
How about make the synchronization points only on call of **createNewLog** in class EntryLogManagerForSingleEntryLog.
Why **createNewLog** can't treat equality?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org