Enhanced DistributedObjectAdmin to compare for inconsistencies between instances #426
Conversation
# Conflicts: # grails-app/controllers/io/xh/hoist/admin/cluster/DistributedObjectAdminController.groovy
| @@ -91,6 +91,8 @@ class ClusterConfig { | |||
| Config createConfig() { | |||
| def ret = new Config() | |||
|
|
|||
| System.out.println("ClusterConfig [INFO] | ${multiInstanceEnabled ? 'Multi-instance is enabled - instances will attempt to cluster.' : 'Multi-instance is disabled - instances will avoid clustering.'}") | |||
There was a problem hiding this comment.
I think trying to mimic the log output is confusing --- can we just log this later in cluster service, when our logging is setup?
| } | ||
| } | ||
|
|
||
| void clearObjects(List<String> names) { |
There was a problem hiding this comment.
Lets call this clearHibernateCaches and be specific! Could potentially combine with method below, or rename method below clearAllHibernateCaches
| } catch (Exception e) { | ||
| def msg = 'Error extracting admin stats' | ||
| logError(msg, e) | ||
| error = "$msg | $e.message" |
There was a problem hiding this comment.
would prefer curly braces on $e.message.
| def resourceObjs = svcs.collectMany { _, svc -> | ||
| [ | ||
| // Services themselves | ||
| getInfo(obj: svc, name: svc.class.getName(), type: 'Service'), |
There was a problem hiding this comment.
think we could clarify with two methods that each just take a single object, and do type switching as needed -- e.g. getHoistInfo(Object obj) and getHzInfo(DistributedObject obj) -- symmetry!
| @@ -185,8 +185,10 @@ class ClusterService extends BaseService implements ApplicationListener<Applicat | |||
| def clazz | |||
| try { | |||
| clazz = Class.forName(Utils.appPackage + '.ClusterConfig') | |||
| System.out.println("ClusterService [INFO] | Found custom ClusterConfig at ${Utils.appPackage + '.ClusterConfig'}") | |||
There was a problem hiding this comment.
again, let's just print out the cluster config package path when logging is available? We have a reference to an instance of the class and can easily print out its full canonical name.
| } | ||
|
|
||
| @Access(['HOIST_ADMIN']) | ||
| def clearObjects() { |
There was a problem hiding this comment.
see below re: naming of these two endpoints to be more specific.
| ] | ||
| } | ||
|
|
||
| List getComparisonFields() { | ||
| if (!replicate) return null |
There was a problem hiding this comment.
super class returned empty list. maybe better to use ternary and return super.getComparisonFields()
| @@ -30,6 +30,8 @@ class CachedValueEntry<T> implements KryoSerializable, LogSupport { | |||
| this.uuid = UUID.randomUUID() | |||
| } | |||
|
|
|||
| static final UNINITIALIZED_CACHE_VALUE_ENTRY = new CachedValueEntry() | |||
There was a problem hiding this comment.
not sure why this change was made, but it worries me that if you ever tried to serialize one of these, I think it would fail with the null loggerName. If we do need this, singleton, I might suggest a different name, and also using the main constructor.
| import static io.xh.hoist.util.Utils.getClusterService | ||
|
|
||
| class DistributedObjectInfo implements JSONFormat { | ||
| // Absolute name of the object, make sure to use `svc.hzName(name)` on relative-named objects |
There was a problem hiding this comment.
I find these interleaved comments make it difficult to read the fairly straightforward property name
| if (a === b) return | ||
|
|
||
| if (!a.isMatching(b)) { | ||
| if (!breaks[name]) breaks[name] = [] |
There was a problem hiding this comment.
withDefault on line 34 makes this a bit cleaner
No description provided.