[
https://issues.apache.org/jira/browse/SOLR-10996?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16076188#comment-16076188
]
Shalin Shekhar Mangar commented on SOLR-10996:
----------------------------------------------
Thanks Andrzej. I reviewed the branch and had a few comments:
# minor nit - AutoScalingConfig.TriggerListenerConfig.stages can be an EnumSet
# String.format used in ScheduledTriggers is a forbidden api because it uses
default locale. Use the overloaded method and pass Locale.ROOT explicitly
# The format string is wrong. You need %s instead of {} for it to work correctly
# We should pass the actual exception to the listener in case of failure of an
action
# We should also pass the ActionContext to the beforeAction and afterAction
listener. The context has important information such as the computed plan etc.
# The fireListeners call should always be guarded by try/catch so that an error
in a listener (for stage=STARTED,BEFORE,AFTER) does not abort processing
actions. Alternately, if a listener fails, should we abort processing and raise
ABORT on all other listeners?
# Should we notify listeners about STARTED at all if the event processor isn't
ready to handle this event? I see that you call listeners with WAITING stage
but should we do that at all? Also, if users have to explicitly subscribe to
WAITING then forgetting to do that can be quite confusing. Imagine receiving
multiple STARTED notifications and no ABORTED/SUCCEEDED/FAILED stages for the
events that had to wait.
# minor nit - ScheduledListeners.TriggerListeners.addPerStage, the conditions
can be replaced with two map.computeIfAbsent calls
# ScheduledListeners.TriggerListeners.close() should clear the listeners list
or else it will leak memory.
# I don't like the idea of a class (TriggerListeners) closing itself. Perhaps
we make it immutable and replace the TriggerListeners instance variable
entirely. Alternately, we do what ScheduledTriggers does with triggers i.e.
close/replace only the listeners which have actually changed. What do you think?
> Implement TriggerListener API
> -----------------------------
>
> Key: SOLR-10996
> URL: https://issues.apache.org/jira/browse/SOLR-10996
> Project: Solr
> Issue Type: Sub-task
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Andrzej Bialecki
> Assignee: Andrzej Bialecki
> Attachments: SOLR-10996.patch
>
>
> SOLR-10340 added API for configuring trigger listeners. This issue is about
> adding hooks in the framework for calling the listeners and providing a
> couple implementations (HTTP callback, logging).
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
