On 18 Oct 2012, at 06:11, Tristan Tarrant wrote:

Hi all,
sorry for this being a bit late (but since it is before CR1 it is not that late :), but the XSite config looks a bit redundant and we should improve it a bit before release

<global>

  ...

  <sites local="LON"/>

  ...

</global>

Since this just provides the local logical name for this site I would instead use

<global>

  ...

  <site name="LON"/>

  ...

</global>

+1, looks better.

In the cache declaration we have the following:

<sites>
     <backups backupSites="NYC,SFO">

       <backup site="NYC" backupFailurePolicy="WARN" strategy="SYNC" timeout="12000"/>

       <backup site="SFO" backupFailurePolicy="IGNORE" strategy="ASYNC"/>

       <backup site="LON" strategy="SYNC"/>

     </backups>

  </sites>

Instead of dumping a list inside an XML attribute (backupSites), can't we just skip that attribute and add an enabled attribute to each backup element ? Also the "backup" prefix to the failure policy is redundant and can be dropped altogether.

I like that more than the list approach, +1.

Also, while I understand the rationale behind giving a "home element" to all xsite-related configurations (<sites>), I do feel it is unnecessary, so I suggest:

     <backups>
       <backup site="NYC" failurePolicy="WARN" strategy="SYNC" timeout="12000" enabled="true"/>

       <backup site="SFO" failurePolicy="IGNORE" strategy="ASYNC"enabled="true"/>

       <backup site="LON" strategy="SYNC" enabled="false"/>

     </backups>

This means that <backupFor/> also moves as a direct child element of a cache declaration.

Not sure sure about this one, I think having all the xsite related configuration grouped under one element kind of makes sense. I don't have strong feelings about it though.

Just my 2¢

Tristan

_______________________________________________
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev