flexiblepower / powermatcher

PowerMatcher - The Java implementation of the PowerMatcher, including the API, the core, a couple of examples, a remote implementation using websockets and a visualisation of the configuration.
http://www.powermatcher.org
Apache License 2.0
41 stars 23 forks source link

Proposed API change: Retrieving status of an Agent #197

Closed marcdejonge closed 9 years ago

marcdejonge commented 9 years ago

This change is needed to make development of agents easier. To get all the concurrent tests running, a lot of locking was needed, because the state of the agent (whether it is connected or not) could change during any function (e.g. when an auctioneer or concentrator is removed). But because locking is hard to get right (and prone to lead to deadlocks) I propose that we can retrieve a snapshot of the state of the agent that can be used during the execution of a function.

For example, quite a few times we have the block that used this kind of pattern

if(isConnected()) {
  // Do something with the marketbasis or clusterId
  getMarketBasis();
  getClusterId();
}

This code could fail if the agent is disconnected after the if-statement (you could get an IllegalStateException if you would call getMarketBasis() or getClusterId()).

What I propose now is the following:

Agent.Status currentStatus = getStatus();
if(currentStatus.isConnected()) {
  // Do something with the marketbasis or clusterId and get them like:
  currentStatus.getMarketBasis();
  currentStatus.getClusterId();
}

This way you are guaranteed that the marketbasis and clusterId is valid. And if you send a new bid while you were disconnected in the meantime, the SessionImpl object will handle that properly.

@alex254 @villgust Can you look at this and accept this if you are OK?

marcdejonge commented 9 years ago

Ps. in this version all the testcases always succeed, so that is at least a big improvement :wink:

villgust commented 9 years ago

I've reviewed the changes and I like the idea of how to solve the concurrency issues. The tests indeed succeed always on my machine as well, so this is very good indeed! ;)

I am wondering about one thing: what are possible issues when the status is retrieved and connections are closed for example? It usually is a short lived process so it will probably be fine. Because it's reference, the status object itself can be replaced without issues.

Still in this case the agent could be working with an (obsolete) session, with a session which are already terminated from the viewpoint of a MatcherEndpoint for example..

marcdejonge commented 9 years ago

This case is handled in the SessionImpl class, because when it is disconnected it will ignore any update calls.