Methods requiring multiple region operations must be transactional [INT-2066]

[spring-operator]

David Turanski opened INT-2066 and commented

MessageGroupStore operations require removing/creating entries in a single UOW must use GF transactions.

---

Affects: 2.1 M1

This issue is a sub-task of #6030

[spring-operator]

David Turanski commented

No longer needed

[spring-operator]

Mark Fisher commented

Can you explain briefly why it's no longer needed?

I'm concerned that the code in #6124 is not transactional when dealing with multiple messages either, because it manipulates the SimpleMessageGroup locally and then re-submits to the region. So, it's possible for overwrites to occur during concurrent operations.

[spring-operator]

Mark Fisher commented

Reopening while awaiting a reply to the last comment.

[spring-operator]

Oleg Zhurakousky commented

It is based on the explanation in #6053. The lock is ensuring serial access to the region (although only in a single AC, but that is a whole different issue). The main point is that there are room for improvements specific to the underlying mechanism. All I was trying to do is to ensure that the style of all MS and MGS is the same for easy maintenance and now it is.

[spring-operator]

Mark Fisher commented

My concern is that since the MessageGroupRegion is being used to get/put the whole group, it increases contention greatly compared to a solution that stores individual instances per-Message that have a reference to their groupId. Then, more than one process could be adding Messages to the group since the granularity is at Message level. Whereas the current code requires that only one process be modifying the entire group at any given time (and as far as I can tell, it does not protect against concurrent writes such that lost updates are possible).

[spring-operator]

Oleg Zhurakousky commented

Agreed and that could be refactored as well. I was actually thinking about it since this is exactly what we do in Redis and Mongo, but my main goal was to make it rely on SimpleMessageGroup, everything else is now internal implementation and could be simplified. In other words it might not be perfect now, but it is better then what it was. . . . i think ;)

[spring-operator]

Mark Fisher commented

I agree it's much better now, but before merging the pull request, we should probably have JIRAs created for those other refactoring issues:

1. distributed locking within the GemfireMessageStore impl (and add one for RedisMessageStore while at it)
2. refactoring GemfireMessageStore to use Message-instance granularity even for the "group" Region

[spring-operator]

Oleg Zhurakousky commented

I am resolving it since we don't have any methods that rely on multiple Regions, however if/when we do we can reopen this one