Add support for custom evictor and trigger policies #2653
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| // Copyright 2017 Twitter. All rights reserved. | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.twitter.heron.api.windowing.triggers; | ||
|
|
||
| import java.io.Serializable; | ||
| import java.util.Map; | ||
|
|
||
| import com.twitter.heron.api.windowing.EvictionPolicy; | ||
| import com.twitter.heron.api.windowing.TriggerHandler; | ||
| import com.twitter.heron.api.windowing.TriggerPolicy; | ||
| import com.twitter.heron.api.windowing.WindowManager; | ||
|
|
||
|
|
||
| public abstract class AbstractBaseTriggerPolicy<T extends Serializable, S> | ||
|
There was a problem hiding this comment. Not sure if this interface is necessary, if we remove WindowManager from the TriggerPolicy interface There was a problem hiding this comment. Regardless, you would still need a way to get the TriggerHandler (WindowPolicy) attached to the custom TriggerPolicy that was passed to the config. AbstractBaseTriggerPolicy was just made as a way to standardize how these attachments were handled for the internal classes, and provide a way to use those same attachment variables in custom classes. |
||
| implements TriggerPolicy<T, S> { | ||
| protected TriggerHandler handler; | ||
| protected EvictionPolicy<T, ?> evictionPolicy; | ||
| protected WindowManager<T> windowManager; | ||
| protected Boolean started; | ||
| protected Map<String, Object> topoConf; | ||
|
|
||
| private boolean requiresEvictionPolicy = false; | ||
| private boolean requiresWindowManager = false; | ||
| private boolean requiresTopologyConfig = false; | ||
|
|
||
| /** | ||
| * Set the requirements in the constructor | ||
| */ | ||
| public AbstractBaseTriggerPolicy(boolean requiresEvictionPolicy, | ||
|
There was a problem hiding this comment. Not sure if we need these checks. I think it should be up to the user to make sure to pass in the correct objects he or she needs for a custom trigger. Having to pass these booleans into the constructor also makes the interface confusing to some reading to code. If you really want these checks in place, I would suggest using setters to set these. Then it is clear to the reader of the code what is being set. There was a problem hiding this comment. @jerrypeng These checks were in place to keep the existing trigger policies as restrictive as they were before wrt which of the objects (evictionPolicy, windowManager) were attached. I could provide an alternate no-arg constructor that sets them all to false, then use setters instead? However, since this is the abstract class which already partially implements the TriggerPolicy interface, I would think it would be okay to perform some checks, but I have no issue with removing them either. There was a problem hiding this comment. lets remove them to keep the interface as simple and clean as possible There was a problem hiding this comment. @jerrypeng will do |
||
| boolean requiresWindowManager, | ||
| boolean requiresTopologyConfig) { | ||
| this.requiresEvictionPolicy = requiresEvictionPolicy; | ||
| this.requiresWindowManager = requiresWindowManager; | ||
| this.requiresTopologyConfig = requiresTopologyConfig; | ||
| } | ||
|
|
||
| /** | ||
| * Set the eviction policy to whatever eviction policy to use this with | ||
| * | ||
| * @param evictionPolicy the eviction policy | ||
| */ | ||
| public void setEvictionPolicy(EvictionPolicy<T, ?> evictionPolicy) { | ||
| this.evictionPolicy = evictionPolicy; | ||
| } | ||
|
|
||
| /** | ||
| * Set the trigger handler for this trigger policy to trigger | ||
| * | ||
| * @param triggerHandler the trigger handler | ||
| */ | ||
| public void setTriggerHandler(TriggerHandler triggerHandler) { | ||
| this.handler = triggerHandler; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the window manager that uses this trigger policy | ||
| * | ||
| * @param windowManager the window manager | ||
| */ | ||
| public void setWindowManager(WindowManager<T> windowManager) { | ||
| this.windowManager = windowManager; | ||
| } | ||
|
|
||
| /** | ||
| * Sets the Config used for this topology | ||
| * | ||
| * @param config the configuration object | ||
| */ | ||
| public void setTopologyConfig(Map<String, Object> config) { | ||
| this.topoConf = config; | ||
| } | ||
|
|
||
| /** | ||
| * Starts the trigger policy. This can be used | ||
| * during recovery to start the triggers after | ||
| * recovery is complete. | ||
| */ | ||
| @Override | ||
| public void start() { | ||
| if (this.evictionPolicy == null && this.requiresEvictionPolicy) { | ||
| throw new RuntimeException("EvictionPolicy of TriggerPolicy was not set."); | ||
| } | ||
|
|
||
| if (this.handler == null) { | ||
| throw new RuntimeException("TriggerHandler of TriggerPolicy was not set."); | ||
| } | ||
|
|
||
| if (this.windowManager == null && this.requiresWindowManager) { | ||
| throw new RuntimeException("WindowManager of TriggerPolicy was not set."); | ||
| } | ||
|
|
||
| if (this.topoConf == null && this.requiresTopologyConfig) { | ||
| throw new RuntimeException("WindowManager of TriggerPolicy was not set."); | ||
| } | ||
|
|
||
| started = true; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Place remove extra line