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***
   
   ![image](https://user-images.githubusercontent.com/12933197/104894609-e0c3a880-59af-11eb-963e-2245f234507c.png)
   
   
   ***To Reproduce***
   ![image](https://user-images.githubusercontent.com/12933197/104894485-be318f80-59af-11eb-8b7e-14a7e4a196af.png)
   
   
   ### 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:
   ![image](https://user-images.githubusercontent.com/12933197/104929911-7b87ab80-59df-11eb-97a6-5cb8e351c722.png)
   ![image](https://user-images.githubusercontent.com/12933197/104929985-8e01e500-59df-11eb-8054-afd29aeca086.png)
   
   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:
   ![image](https://user-images.githubusercontent.com/12933197/104929911-7b87ab80-59df-11eb-97a6-5cb8e351c722.png)
   ![image](https://user-images.githubusercontent.com/12933197/104929985-8e01e500-59df-11eb-8054-afd29aeca086.png)
   
   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 
   ![image](https://user-images.githubusercontent.com/12933197/104933198-94925b80-59e3-11eb-9954-571fa5f8963f.png)
   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
   ![image](https://user-images.githubusercontent.com/12933197/104974427-f201c900-5a32-11eb-9ffd-7e572128e919.png)
   


----------------------------------------------------------------
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
   ![image](https://user-images.githubusercontent.com/12933197/104974427-f201c900-5a32-11eb-9ffd-7e572128e919.png)
   


----------------------------------------------------------------
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