Support filtering refUpdate events for NoteDb meta refs Ref update events on notedb meta refs are not useful and likely won't ever be consumed by external users as it is an internal Gerrit ref. Add support for a DROP rule to filter them out: DROP RefUpdatedEvent isNoteDbMetaRef On our busy servers with too many writes on changes it seems to take too long to store events, this sometimes causes other writes to suffer so hopefully filtering these out will help. Release-Notes: NoteDb refUpdatedEvents can now be filtered Change-Id: I813d085d8a0ea5c6cdeb83753f97e8ee50727370
diff --git a/src/main/java/com/googlesource/gerrit/plugins/events/FileSystemEventBroker.java b/src/main/java/com/googlesource/gerrit/plugins/events/FileSystemEventBroker.java index cdba933..2791a19 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/events/FileSystemEventBroker.java +++ b/src/main/java/com/googlesource/gerrit/plugins/events/FileSystemEventBroker.java
@@ -18,6 +18,7 @@ import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.Project; +import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.server.CurrentUser; @@ -46,6 +47,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.function.Predicate; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,15 +55,25 @@ public class FileSystemEventBroker extends EventBroker { private static final Logger log = LoggerFactory.getLogger(FileSystemEventBroker.class); + protected final static Predicate<Event> IS_NOTEDB_METAREF = event -> { + if (event instanceof RefEvent) { + return RefNames.isNoteDbMetaRef(((RefEvent) event).getRefName()); + } + return false; + }; + protected static final String KEY_FILTER = "filter"; protected static final String FILTER_TYPE_DROP = "DROP"; protected static final String FILTER_ELEMENT_CLASSNAME = "classname"; + protected static final String FILTER_ELEMENT_EVENT_REFUPDATED = "RefUpdatedEvent"; + protected static final String FILTER_TEST_IS_NOTEDB_METAREF = "isNoteDbMetaRef"; protected final EventStore store; protected final Gson gson; protected final DynamicSet<StreamEventListener> streamEventListeners; protected long lastSent; + protected Predicate<Event> drop = e -> false; protected Set<String> dropEventNames = new HashSet<>(); @Inject @@ -126,14 +138,21 @@ } protected void storeEvent(Event event) { - if (dropEventNames.contains(event.getClass().getName())) { - return; + if (!isDropEvent(event)) { + try { + store.add(gson.toJson(event)); + } catch (IOException ex) { + log.error("Cannot add event to event store", ex); + } } - try { - store.add(gson.toJson(event)); - } catch (IOException ex) { - log.error("Cannot add event to event store", ex); + } + + protected boolean isDropEvent(Event event) { + if (drop.test(event) || + dropEventNames.contains(event.getClass().getName())) { + return true; } + return false; } public synchronized void fireEventForStreamListeners() throws PermissionBackendException { @@ -230,13 +249,20 @@ PluginConfig cfg = PluginConfig.createFromGerritConfig(pluginName, configProvider.loadConfig()); for (String filter : cfg.getStringList(KEY_FILTER)) { String pieces[] = filter.split(" "); - if (pieces.length == 3 - && FILTER_TYPE_DROP.equals(pieces[0]) - && FILTER_ELEMENT_CLASSNAME.equals(pieces[1])) { - dropEventNames.add(pieces[2]); - } else { - log.error("Ignoring invalid filter: " + filter); + if (pieces.length == 3) { + if (FILTER_TYPE_DROP.equals(pieces[0])) { + if (FILTER_ELEMENT_CLASSNAME.equals(pieces[1])) { + dropEventNames.add(pieces[2]); + continue; + } + if (FILTER_ELEMENT_EVENT_REFUPDATED.equals(pieces[1]) + && FILTER_TEST_IS_NOTEDB_METAREF.equals(pieces[2])) { + drop = IS_NOTEDB_METAREF; + continue; + } + } } + log.error("Ignoring invalid filter: " + filter); } } }
diff --git a/src/main/resources/Documentation/about.md b/src/main/resources/Documentation/about.md index f4d8079..c34eea7 100644 --- a/src/main/resources/Documentation/about.md +++ b/src/main/resources/Documentation/about.md
@@ -27,6 +27,10 @@ DROP classname fully.qualified.java.ClassName +or: + + DROP RefUpdatedEvent isNoteDbMetaRef + If the `plugin.@PLUGIN@.filter` key is specified more than once it will cause events matching any of the rules to be dropped.
diff --git a/test/test_events_plugin.sh b/test/test_events_plugin.sh index 3205e34..abf046e 100755 --- a/test/test_events_plugin.sh +++ b/test/test_events_plugin.sh
@@ -107,6 +107,13 @@ --data '{"message":"unmark_private"}' "$REST_API_CHANGES_URL/$1/private" } + +add_meta_ref_updates() { # num_events num_meta_ref_upates > total_num_events + local n=$1 m=$2 + [ "$FILTERED" = "META_REF_UPDATES" ] && { echo "$n" ; return ; } + echo $(($n + $m)) +} + # ------------------------- Event Capturing --------------------------- kill_diff_captures() { # sig @@ -178,6 +185,68 @@ result_type_for core "$@" } +# ------------------------- Tests --------------------------- + +main_suite() { # group_name + GROUP=$1 + setup_diff_captures + + type=patchset-created + capture_events $(add_meta_ref_updates 2 1) + ch1=$(create_change "$REF_BRANCH" "$FILE_A") || exit + result_type "$GROUP" "$type" 1 + # The change ref and its meta ref are expected to be updated + # For example: 'refs/changes/01/1001/1' and 'refs/changes/01/1001/meta' + result_type "$GROUP $type" "ref-updated" $(add_meta_ref_updates 1 1) + + type=change-abandoned + capture_events $(add_meta_ref_updates 1 2) + review "$ch1,1" --abandon + result_type "$GROUP" "$type" + + type=change-restored + capture_events $(add_meta_ref_updates 1 1) + review "$ch1,1" --restore + result_type "$GROUP" "$type" + + type=comment-added + capture_events $(add_meta_ref_updates 1 1) + review "$ch1,1" --message "my_comment" $APPROVALS + result_type "$GROUP" "$type" + + type=wip-state-changed + capture_events $(add_meta_ref_updates 1 3) + q mark_change_wip "$ch1" + q mark_change_ready "$ch1" + result_type "$GROUP" "$type" $(add_meta_ref_updates 1 1) + + type=private-state-changed + capture_events $(add_meta_ref_updates 1 3) + q mark_change_private "$ch1" + q unmark_change_private "$ch1" + result_type "$GROUP" "$type" $(add_meta_ref_updates 1 1) + + type=change-merged + events_count=$(add_meta_ref_updates 2 2) + # If reviewnotes plugin is installed, an extra event of type 'ref-updated' + # on 'refs/notes/review' is fired when a change is merged. + is_plugin_installed reviewnotes && events_count="$((events_count+1))" + capture_events "$events_count" + submit "$ch1,1" + result_type "$GROUP" "$type" + # The destination ref of the change, its meta ref and notes ref(if reviewnotes + # plugin is installed) are expected to be updated. + # For example: 'refs/heads/master', 'refs/changes/01/1001/meta' and 'refs/notes/review' + result_type "$GROUP $type" "ref-updated" "$((events_count-1))" + + # reviewer-added needs to be tested via Rest-API + + out=$(diff -- "$EVENTS_CORE" "$EVENTS_PLUGIN") + result "$GROUP core/plugin diff" "$out" + + kill_diff_captures +} + # ------------------------- Usage --------------------------- usage() { # [error_message] @@ -266,79 +335,29 @@ RESULT=0 # ------------------------- Individual Event Tests --------------------------- -GROUP=visible-events +FILTERED="" set_filter_rules # No rules -setup_diff_captures - -type=patchset-created -capture_events 3 -ch1=$(create_change "$REF_BRANCH" "$FILE_A") || exit -result_type "$GROUP" "$type" 1 -# The change ref and its meta ref are expected to be updated -# For example: 'refs/changes/01/1001/1' and 'refs/changes/01/1001/meta' -result_type "$GROUP $type" "ref-updated" 2 - -type=change-abandoned -capture_events 3 -review "$ch1,1" --abandon -result_type "$GROUP" "$type" - -type=change-restored -capture_events 2 -review "$ch1,1" --restore -result_type "$GROUP" "$type" - -type=comment-added -capture_events 2 -review "$ch1,1" --message "my_comment" $APPROVALS -result_type "$GROUP" "$type" - -type=wip-state-changed -capture_events 4 -q mark_change_wip "$ch1" -q mark_change_ready "$ch1" -result_type "$GROUP" "$type" 2 - -type=private-state-changed -capture_events 4 -q mark_change_private "$ch1" -q unmark_change_private "$ch1" -result_type "$GROUP" "$type" 2 - -type=change-merged -events_count=4 -# If reviewnotes plugin is installed, an extra event of type 'ref-updated' -# on 'refs/notes/review' is fired when a change is merged. -is_plugin_installed reviewnotes && events_count="$((events_count+1))" -capture_events "$events_count" -submit "$ch1,1" -result_type "$GROUP" "$type" -# The destination ref of the change, its meta ref and notes ref(if reviewnotes -# plugin is installed) are expected to be updated. -# For example: 'refs/heads/master', 'refs/changes/01/1001/meta' and 'refs/notes/review' -result_type "$GROUP $type" "ref-updated" "$((events_count-1))" - -# reviewer-added needs to be tested via Rest-API - -out=$(diff -- "$EVENTS_CORE" "$EVENTS_PLUGIN") -result "$GROUP core/plugin diff" "$out" - -kill_diff_captures +main_suite visible-events # ------------------------- Filtering ------------------------- +FILTERED="META_REF_UPDATES" +set_filter_rules 'DROP RefUpdatedEvent isNoteDbMetaRef' +main_suite meta-refUpdated-filtered + + +FILTERED="" GROUP=restored-filtered set_filter_rules 'DROP classname com.google.gerrit.server.events.ChangeRestoredEvent' ch1=$(create_change "$REF_BRANCH" "$FILE_A") || exit type=change-abandoned - -capture_events 3 +capture_events $(add_meta_ref_updates 1 2) review "$ch1,1" --abandon result_type "$GROUP" "$type" type=change-restored -capture_events 4 +capture_events $(add_meta_ref_updates 3 1) review "$ch1,1" --restore # Instead of timing out waiting for the filtered change-restored event, # create follow-on events and capture them to trigger completion.