[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/2483 ---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r245090041
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -309,16 +309,17 @@ public void run() {
*/
@Override
protected void performCachedLargeMessageDeletes() {
- for (Long largeMsgId : largeMessagesToDelete) {
- SequentialFile msg = createFileForLargeMessage(largeMsgId,
LargeMessageExtension.DURABLE);
+ for (LargeServerMessage largeServerMessage :
largeMessagesToDelete.values()) {
--- End diff --
Usage of LongConcurrentHashMap looks much better.
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244977454
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -727,7 +725,7 @@ private void checkAndCreateDir(final File dir, final
boolean create) {
List idList = new ArrayList<>();
for (String filename : filenames) {
Long id = getLargeMessageIdFromFilename(filename);
--- End diff --
you can just use a primitive `long` here
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user CNNJYB commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244976446
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -309,16 +309,17 @@ public void run() {
*/
@Override
protected void performCachedLargeMessageDeletes() {
- for (Long largeMsgId : largeMessagesToDelete) {
- SequentialFile msg = createFileForLargeMessage(largeMsgId,
LargeMessageExtension.DURABLE);
+ for (LargeServerMessage largeServerMessage :
largeMessagesToDelete.values()) {
--- End diff --
@michaelandrepearce update, please review, Thanks.
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244757759
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -309,16 +309,17 @@ public void run() {
*/
@Override
protected void performCachedLargeMessageDeletes() {
- for (Long largeMsgId : largeMessagesToDelete) {
- SequentialFile msg = createFileForLargeMessage(largeMsgId,
LargeMessageExtension.DURABLE);
+ for (LargeServerMessage largeServerMessage :
largeMessagesToDelete.values()) {
--- End diff --
If you wish the collection itself to be iterable, then please add this
functionality to LongConcurrentHashMap implementation, it shouldn;t be too
hard, as already it has a forEach method
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244756852
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -309,16 +309,17 @@ public void run() {
*/
@Override
protected void performCachedLargeMessageDeletes() {
- for (Long largeMsgId : largeMessagesToDelete) {
- SequentialFile msg = createFileForLargeMessage(largeMsgId,
LargeMessageExtension.DURABLE);
+ for (LargeServerMessage largeServerMessage :
largeMessagesToDelete.values()) {
--- End diff --
Calling values actually creates a new List, if you're iterating the
objects, simply call using forEach method on the collection.
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244686021
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java
---
@@ -193,7 +193,7 @@ public static JournalContent getType(byte type) {
protected final Map
mapPersistedAddressSettings = new ConcurrentHashMap<>();
- protected final Set largeMessagesToDelete = new HashSet<>();
+ protected final Map largeMessagesToDelete =
new HashMap<>();
--- End diff --
@CNNJYB we have our own
org.apache.activemq.artemis.utils.collections.LongConcurrentHashMap, this
allows it to be concurrent safe, and also means it can be a primitive long.
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user michaelandrepearce commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244685607
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -461,8 +462,7 @@ void deleteLargeMessageFile(final LargeServerMessage
largeServerMessage) throws
try {
if (isReplicated() && replicator.isSynchronizing()) {
synchronized (largeMessagesToDelete) {
--- End diff --
@franz1981 looking at the code, looks like its just vanilla HM
protected final Map largeMessagesToDelete =
new HashMap<>();
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user CNNJYB commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244657469
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java
---
@@ -193,7 +193,7 @@ public static JournalContent getType(byte type) {
protected final Map
mapPersistedAddressSettings = new ConcurrentHashMap<>();
- protected final Set largeMessagesToDelete = new HashSet<>();
+ protected final Map largeMessagesToDelete =
new ConcurrentHashMap<>();
--- End diff --
@franz1981 modified, please review, Thanks.
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244470771
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/JournalStorageManager.java
---
@@ -461,8 +462,7 @@ void deleteLargeMessageFile(final LargeServerMessage
largeServerMessage) throws
try {
if (isReplicated() && replicator.isSynchronizing()) {
synchronized (largeMessagesToDelete) {
--- End diff --
If it's now using CHM there is no need to sync on it
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
Github user franz1981 commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2483#discussion_r244470859
--- Diff:
artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/AbstractJournalStorageManager.java
---
@@ -193,7 +193,7 @@ public static JournalContent getType(byte type) {
protected final Map
mapPersistedAddressSettings = new ConcurrentHashMap<>();
- protected final Set largeMessagesToDelete = new HashSet<>();
+ protected final Map largeMessagesToDelete =
new ConcurrentHashMap<>();
--- End diff --
It is possible to use a primitive version of the map ie using primitive
longs instead of boxed types
---
[GitHub] activemq-artemis pull request #2483: ARTEMIS-2215 largemessage have been con...
GitHub user CNNJYB opened a pull request: https://github.com/apache/activemq-artemis/pull/2483 ARTEMIS-2215 largemessage have been consumed but not deleted During the backup and live synchronization, the client consumes the largemessage, then the live crash(the performCachedLargeMessageDeletes method is not executed), after the live startup, the largemessages that have been consumed are not deleted from the disk. You can merge this pull request into a Git repository by running: $ git pull https://github.com/CNNJYB/activemq-artemis dev-largemessagenotdelete Alternatively you can review and apply these changes as the patch at: https://github.com/apache/activemq-artemis/pull/2483.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #2483 commit bf28c751af152956d1a12aa2237502c0a14fc4e8 Author: yb <17061955@...> Date: 2018-12-29T08:09:48Z ARTEMIS-2215 largemessage have been consumed but not deleted from the disk during backup and live sync ---
