Reactivating SharedStoreCacheLoader

[galder.zamarreno]

As per conversations with Bela and Manik, I'm planning to have a go at
http://jira.jboss.com/jira/browse/JBCACHE-650 which refers to
reactivating SharedStoreCacheLoader. Had a quick look at it and this is
what I think needs to be done:

- Refactor SharedStoreCacheLoader to SingletonCacheLoader
- Adding a tag to cacheloader element called, for example, singleton?
This'd be a similar to what it was done with AsyncCacheLoader.
- Enable, via configuration, dumping of the in-memory state when a node
becomes the coordinator. Maybe using a pushStateWhenCoordinator
attribute in the singleton element (defaul value would be true)? i.e.

<singleton pushStateWhenCoordinator="true">

- Configuration changes will need to be backported to 1.2.x using the old
CacheLoader configuration style (http://wiki.jboss.org/wiki/Wiki.jsp?page=JBossCacheCacheLoaders)
- viewChange in SingletonCacheLoader would have to change in order to
transfer the in-memory state into the underlying cache loader. Haven't
looked at the specific details of this, but I expect to do that in the next few
days. Just wanted to start the discussion.
- Validate that singleton is not used when the cache has been configured
with evictions.

This JIRA task is also very similar to http://jira.jboss.com/jira/browse/JBCACHE-623.
We probably wanna merge them into one? JBCACHE-623 mentions the
configuration requirements and the need to dump the in-memory state
when changing the coordinator. But it also adds the fact that
CacheLoaderShared needs to be false, which makes sense anyway.

[manik]

- Configuration changes will need to be backported to 1.2.x using the old
CacheLoader configuration style (http://wiki.jboss.org/wiki/Wiki.jsp?page=JBossCacheCacheLoaders )

Is this necessary? The SingletonCacheLoader won't be available till JBC 2.x.x anyway.

[manik]

Just so we don't destabilise the 1.4.0 release, I'd hold back on any implementations of this until I branch 1.4.0 (probably over the next few days, once I release .CR1).

[galder.zamarreno]

- Configuration changes will need to be backported to 1.2.x using the old
CacheLoader configuration style (http://wiki.jboss.org/wiki/Wiki.jsp?page=JBossCacheCacheLoaders )

Is this necessary? The SingletonCacheLoader won't be available till JBC 2.x.x anyway.

As the affected versions included 1.2.x, I thought we might wanna back port it .

[galder.zamarreno]

Just so we don't destabilise the 1.4.0 release, I'd hold back on any implementations of this until I branch 1.4.0 (probably over the next few days, once I release .CR1).

no probs, I'm just starting to have a look to it :-)

[manik]

-1 on backporting. People can upgrade if they need it.

[galder.zamarreno]

that's alright with me, it was just a misunderstanding

[belaban]

+1 from me. I guess this needs to be done on the 1.4 branch *and* 2.0 (CVS head) ?

Also +1 on the name change to SingletonCacheLoader

[manik]

Just in HEAD (for 2.0.0). This is not targetted for the 1.4.0 release.

[galder.zamarreno]

I had a bit more look to this and I have this questions:

* Looking at SharedStoreCacheLoader, it seems like it's the cache loader itself that, given a org.jgroups.View and an org.jgroups.Address, decides whether that Address is the coordinator of the View. I don't think it should change now, but don't you think this operation should live in JGroups rather than in JBossCache? JGroups knows these two and I think it should be responsible of being able to determine whether an Address is the coordinator of a View:

Code in SharedStoreCacheLoader:

public void viewChange(View new_view) {
 boolean tmp=active;
 if(new_view != null && local_addr != null) {
 Vector mbrs=new_view.getMembers();
 if(mbrs != null && mbrs.size() > 0 && local_addr.equals(mbrs.firstElement())) {
 tmp=true;
 }
 else {
 tmp=false;
 }
 }
 if(active != tmp) {
 active=tmp;
 log.info("changed mode: active=" + active);
 }
 }

* Configuration wise, I had this two ideas, but I'm not sure which one is preferred, or if there's any other:

<singleton>true</singleton>
<singleton>false</singleton> (default)

<singleton pushStateWhenCoordinator="true">true</singleton> (pushStateWhenCoordinator defaults to true)
<singleton pushStateWhenCoordinator="false">true</singleton>

OR:

<singleton />

<singleton>
 <pushStateWhenCoordinator>true</pushStateWhenCoordinator>
</singleton>
<singleton>
 <pushStateWhenCoordinator>false</pushStateWhenCoordinator>
</singleton>

[galder.zamarreno]

"this questions"
"this two ideas"

I think i might need some spelling lessons!

[galder.zamarreno]

Configuration wise, personally, I think pushStateWhenCoordinator should be an attribute of the singleton element. Opinions?

[belaban]

I think this should be done in JBossCache, you have a list of addresses, the coordinator is always the first member...

[galder.zamarreno]

I guess it'd be easier to do View.getVid().getCoordAddress()?

[galder.zamarreno]

I've been a bit productive this weekend and implemented the configuration side and unit tested the SingletonCacheLoader using a DummyCacheLoader as the delegated cache loader. The original code seems to working fine :-).

Configuration wise, I've gone for the element/attribute pattern. In terms of validation, singleton should not be used neither with shared nor with evictions as it would not make sense at all. Therefore, I have added validations to check this on startup.

Now, in order to get the pushStateWhenCoordinator feature done, the only method that I can use seems to be:
TreeCache._getState(Fqn fqn, boolean fetchTransientState, boolean fetchPersistentState, long timeout, boolean force, boolean suppressErrors)

fetchTransientState would be true and fetchPesistentState false, however, I am not sure what timeout should I use. Should I used a hardcoded value or should I add to the configuration? Also, I guess force should be hardcoded to false.