An aggregate is being registered with this UnitOfWork more than once - In this really an issue

Imported from Google Groups
#1

When handling command we a getting alot of the following log messages

WARN org.axonframework.unitofwork.DefaultUnitOfWork An aggregate is
being registered with this UnitOfWork more than once. Although this is
not likely to cause problems, it is improper use of resources.

After looking at the code is appears that the we are loading aggregate
more than once in the processing of a command. Is order to reduce hit
to the database I was considereing using the caching respositories.
Unfortunately there is no SpringPrototypeCachingRepository so I will
probably roll my own.

My question is should axon provide a caching capability within the
UnitOfWork. This would be similar to the hibernate Session level
caching. I see the Caching repository implementations as a second
level caching mechanism.

#2

Hi,

Axon already does that. But loading the same aggregate within a single unit of work is not good practice, and might be an indication that something is not as expected. That’s why (I hope) your application works, but the anomaly is reported in the log files. If you have a situation where you can’t get round this issue, I’m eager to know more about it.

I will look into caching for the SpringPrototypeRepository as well.

Cheers,

Allard

#3

Hi Allard,

From what I can see in AbsractRepository class if I was not using a caching repository implementation then each call to the AbractRepository.load which then calls the abstract AbractRepository.doLoad method hitting the database. The CurrentUnitOfWork does not seem to be used a first level cache. I think :slight_smile:

The reason we are seeing the WARN message repeatedly in our logs is that we are using a graph structure within our domain as are traversing it (we use jgrapht). The traversal code is basically a class that implements a TraversalListener. Each time the traversal algorithm encouters a vertex we perform some processing that involves a lookup on the Repository. In order to keep these traversal classes independent we simply inject the Repository. There are situations where the same Aggregates is loaded in the traversal more that once as the traversal visits each vertex. I could cache them myself somehow to avoid call the Repository.load() but I see this as a common problem solved by a first level in the currentUnitOfWork. Obviously using the caching reposistory implementation reduces the database hits however the WARN message seems wrong in our case.

Shea

#4

Hi Shea,

the first level cache is in the DefaultUnitOfWork class. The first line of the “registerAggregate” method will check if an aggregate of the same class and identifier has already been registered. I am reconsidering that warning it is outputting. Initially, I thought duplicate registrations of aggregates could lead to resources not being freed correctly. However, resources are freed using other callbacks.

Long story short: your code works as expected and I will change the warning into an informative message (on info level).
Thanks for your view on this!

Cheers,

Allard

#5

Thanks Allard

The code I am referring to is below taken from the AbstractRepository. The first line doLoad is calling in our case the EventSourcingRepository.doLoad implementation which then calls the JpaEventStore.
I can see how the DefaultUnitOfWork checks before registering the aggregate twice, however I think the current aggregate should be loaded from the DefaultUnitOfWork first before it hits the eventStore

@Override
public T load(AggregateIdentifier aggregateIdentifier, Long expectedVersion) {
T aggregate = doLoad(aggregateIdentifier, expectedVersion);
validateOnLoad(aggregate, expectedVersion);
return CurrentUnitOfWork.get().registerAggregate(aggregate, saveAggregateCallback);
}

#6

This is the moment where I think, “whoops” and realize that you’re 100% right. The UnitOfWork is not acting as a first level cache, but just as a “only keep a single instance of equal Aggregates” kind of thing.

I guess that it would be safe to skip the doLoad(…) method if an aggregate is already in the UnitOfWork. However, I will have to find a not-too-intrusive way to get the type of the aggregate before it is being loaded. The repository knows which type it is serving, so it shouldn’t be too hard to do.

I’ll have a look at it very soon.

Cheers,

Allard

#7

It may want to be based on the aggregate ID, with no warning if the objects are == when registered.

There really wants to be a warning if there are two different objects registered for the same aggregate ID (even if they are .equals())

  • Stan
#8

The problem with the aggregateId is that there may potentially be different aggregates of different types with the same identifier. Absolute uniqueness is achieved by combining aggregate type and aggregate identifier. To use the UnitOfWork as a 1st level cache, it would need to check for an aggregate of the same type and same identifier.

The problem I have currently with implementing this, is that the AbstractRepository is not aware of the type it is serving. There is some generic typing on it, but that is erased at Runtime. I’ll have to change the API a littlebit to accomodate this. I’m not sure it will be included in the 1.1 release. I don’t want to make any API changes there.

A small side note: I wouldn’t implement the equals (nor hashCode) methods on aggregates. In fact, I only implement them on value objects. I wrote a blog about that a while ago, if you’re interested: http://www.gridshore.nl/2009/07/29/domain-driven-design-and-the-equals-method/

Cheers,

Allard

#9

Hi Allard,

Regarding the problem "the AbstractRepository is not aware of the type it is serving. ", I read this article http://www.artima.com/weblogs/viewpost.jsp?thread=208860 and I implement the tip in one of my framework test. Thus it forces me to add another super class. But in your case you already implements Repository in AbstractRepository .

So just adding this method to AbstractRepository did the tricks, i tested it against the code.

public Class returnedClass() {
Class clazz = null;
ParameterizedType parameterizedType =
(ParameterizedType) getClass().getGenericSuperclass();
clazz = (Class) parameterizedType.getActualTypeArguments()[0];
return clazz;
}

As said in the article, this code works for normal class (no array ). The author give some hints to return all kind of class but I think for the axon framework Aggregate class are POJO with no complexity.

Hope this helps

Epo

2011/5/10 Allard Buijze <buijze@gmail.com>