Committed by
Gerrit Code Review
Enhancing accumulator to allow subclasses to indicate whether they are ready for…
… the batch to be processed. Default behaviour returns true. Change-Id: I53a3ffc3ecd75ed2607f155a61971e05a6009a66
Showing
3 changed files
with
80 additions
and
12 deletions
| ... | @@ -76,7 +76,7 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { | ... | @@ -76,7 +76,7 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { |
| 76 | items.add(checkNotNull(item, "Item cannot be null")); | 76 | items.add(checkNotNull(item, "Item cannot be null")); |
| 77 | 77 | ||
| 78 | // Did we hit the max item threshold? | 78 | // Did we hit the max item threshold? |
| 79 | - if (items.size() == maxItems) { | 79 | + if (items.size() >= maxItems) { |
| 80 | maxTask = cancelIfActive(maxTask); | 80 | maxTask = cancelIfActive(maxTask); |
| 81 | schedule(1); | 81 | schedule(1); |
| 82 | } else { | 82 | } else { |
| ... | @@ -108,12 +108,16 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { | ... | @@ -108,12 +108,16 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { |
| 108 | private class ProcessorTask extends TimerTask { | 108 | private class ProcessorTask extends TimerTask { |
| 109 | @Override | 109 | @Override |
| 110 | public void run() { | 110 | public void run() { |
| 111 | - try { | 111 | + idleTask = cancelIfActive(idleTask); |
| 112 | - idleTask = cancelIfActive(idleTask); | 112 | + if (isReady()) { |
| 113 | - maxTask = cancelIfActive(maxTask); | 113 | + try { |
| 114 | - processItems(finalizeCurrentBatch()); | 114 | + maxTask = cancelIfActive(maxTask); |
| 115 | - } catch (Exception e) { | 115 | + processItems(finalizeCurrentBatch()); |
| 116 | - log.warn("Unable to process batch due to {}", e); | 116 | + } catch (Exception e) { |
| 117 | + log.warn("Unable to process batch due to {}", e); | ||
| 118 | + } | ||
| 119 | + } else { | ||
| 120 | + idleTask = schedule(maxIdleMillis); | ||
| 117 | } | 121 | } |
| 118 | } | 122 | } |
| 119 | } | 123 | } |
| ... | @@ -125,6 +129,11 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { | ... | @@ -125,6 +129,11 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { |
| 125 | return toBeProcessed; | 129 | return toBeProcessed; |
| 126 | } | 130 | } |
| 127 | 131 | ||
| 132 | + @Override | ||
| 133 | + public boolean isReady() { | ||
| 134 | + return true; | ||
| 135 | + } | ||
| 136 | + | ||
| 128 | /** | 137 | /** |
| 129 | * Returns the backing timer. | 138 | * Returns the backing timer. |
| 130 | * | 139 | * |
| ... | @@ -163,4 +172,5 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { | ... | @@ -163,4 +172,5 @@ public abstract class AbstractAccumulator<T> implements Accumulator<T> { |
| 163 | public int maxIdleMillis() { | 172 | public int maxIdleMillis() { |
| 164 | return maxIdleMillis; | 173 | return maxIdleMillis; |
| 165 | } | 174 | } |
| 175 | + | ||
| 166 | } | 176 | } | ... | ... |
| ... | @@ -40,6 +40,10 @@ public interface Accumulator<T> { | ... | @@ -40,6 +40,10 @@ public interface Accumulator<T> { |
| 40 | */ | 40 | */ |
| 41 | void processItems(List<T> items); | 41 | void processItems(List<T> items); |
| 42 | 42 | ||
| 43 | - //TODO consider a blocking version that required consumer participation | 43 | + /** |
| 44 | - | 44 | + * Indicates whether the accumulator is ready to process items. |
| 45 | + * | ||
| 46 | + * @return true if ready to process | ||
| 47 | + */ | ||
| 48 | + boolean isReady(); | ||
| 45 | } | 49 | } | ... | ... |
| ... | @@ -37,7 +37,7 @@ public class AbstractAccumulatorTest { | ... | @@ -37,7 +37,7 @@ public class AbstractAccumulatorTest { |
| 37 | assertEquals("incorrect timer", timer, accumulator.timer()); | 37 | assertEquals("incorrect timer", timer, accumulator.timer()); |
| 38 | assertEquals("incorrect max events", 5, accumulator.maxItems()); | 38 | assertEquals("incorrect max events", 5, accumulator.maxItems()); |
| 39 | assertEquals("incorrect max ms", 100, accumulator.maxBatchMillis()); | 39 | assertEquals("incorrect max ms", 100, accumulator.maxBatchMillis()); |
| 40 | - assertEquals("incorrect idle ms", 50, accumulator.maxIdleMillis()); | 40 | + assertEquals("incorrect idle ms", 70, accumulator.maxIdleMillis()); |
| 41 | } | 41 | } |
| 42 | 42 | ||
| 43 | @Test | 43 | @Test |
| ... | @@ -68,7 +68,7 @@ public class AbstractAccumulatorTest { | ... | @@ -68,7 +68,7 @@ public class AbstractAccumulatorTest { |
| 68 | delay(30); | 68 | delay(30); |
| 69 | assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | 69 | assertTrue("should not have fired yet", accumulator.batch.isEmpty()); |
| 70 | accumulator.add(new TestItem("d")); | 70 | accumulator.add(new TestItem("d")); |
| 71 | - delay(30); | 71 | + delay(60); |
| 72 | assertFalse("should have fired", accumulator.batch.isEmpty()); | 72 | assertFalse("should have fired", accumulator.batch.isEmpty()); |
| 73 | assertEquals("incorrect batch", "abcd", accumulator.batch); | 73 | assertEquals("incorrect batch", "abcd", accumulator.batch); |
| 74 | } | 74 | } |
| ... | @@ -84,6 +84,54 @@ public class AbstractAccumulatorTest { | ... | @@ -84,6 +84,54 @@ public class AbstractAccumulatorTest { |
| 84 | assertEquals("incorrect batch", "ab", accumulator.batch); | 84 | assertEquals("incorrect batch", "ab", accumulator.batch); |
| 85 | } | 85 | } |
| 86 | 86 | ||
| 87 | + @Test | ||
| 88 | + public void readyIdleTrigger() { | ||
| 89 | + TestAccumulator accumulator = new TestAccumulator(); | ||
| 90 | + accumulator.ready = false; | ||
| 91 | + accumulator.add(new TestItem("a")); | ||
| 92 | + assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | ||
| 93 | + accumulator.add(new TestItem("b")); | ||
| 94 | + delay(80); | ||
| 95 | + assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | ||
| 96 | + accumulator.ready = true; | ||
| 97 | + delay(80); | ||
| 98 | + assertFalse("should have fired", accumulator.batch.isEmpty()); | ||
| 99 | + assertEquals("incorrect batch", "ab", accumulator.batch); | ||
| 100 | + } | ||
| 101 | + | ||
| 102 | + @Test | ||
| 103 | + public void readyLongTrigger() { | ||
| 104 | + TestAccumulator accumulator = new TestAccumulator(); | ||
| 105 | + accumulator.ready = false; | ||
| 106 | + delay(120); | ||
| 107 | + assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | ||
| 108 | + accumulator.add(new TestItem("a")); | ||
| 109 | + assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | ||
| 110 | + accumulator.ready = true; | ||
| 111 | + delay(80); | ||
| 112 | + assertFalse("should have fired", accumulator.batch.isEmpty()); | ||
| 113 | + assertEquals("incorrect batch", "a", accumulator.batch); | ||
| 114 | + } | ||
| 115 | + | ||
| 116 | + @Test | ||
| 117 | + public void readyMaxTrigger() { | ||
| 118 | + TestAccumulator accumulator = new TestAccumulator(); | ||
| 119 | + accumulator.ready = false; | ||
| 120 | + accumulator.add(new TestItem("a")); | ||
| 121 | + accumulator.add(new TestItem("b")); | ||
| 122 | + accumulator.add(new TestItem("c")); | ||
| 123 | + accumulator.add(new TestItem("d")); | ||
| 124 | + accumulator.add(new TestItem("e")); | ||
| 125 | + accumulator.add(new TestItem("f")); | ||
| 126 | + assertTrue("should not have fired yet", accumulator.batch.isEmpty()); | ||
| 127 | + accumulator.ready = true; | ||
| 128 | + accumulator.add(new TestItem("g")); | ||
| 129 | + delay(5); | ||
| 130 | + assertFalse("should have fired", accumulator.batch.isEmpty()); | ||
| 131 | + assertEquals("incorrect batch", "abcdefg", accumulator.batch); | ||
| 132 | + } | ||
| 133 | + | ||
| 134 | + | ||
| 87 | private class TestItem { | 135 | private class TestItem { |
| 88 | private final String s; | 136 | private final String s; |
| 89 | 137 | ||
| ... | @@ -95,9 +143,10 @@ public class AbstractAccumulatorTest { | ... | @@ -95,9 +143,10 @@ public class AbstractAccumulatorTest { |
| 95 | private class TestAccumulator extends AbstractAccumulator<TestItem> { | 143 | private class TestAccumulator extends AbstractAccumulator<TestItem> { |
| 96 | 144 | ||
| 97 | String batch = ""; | 145 | String batch = ""; |
| 146 | + boolean ready = true; | ||
| 98 | 147 | ||
| 99 | protected TestAccumulator() { | 148 | protected TestAccumulator() { |
| 100 | - super(timer, 5, 100, 50); | 149 | + super(timer, 5, 100, 70); |
| 101 | } | 150 | } |
| 102 | 151 | ||
| 103 | @Override | 152 | @Override |
| ... | @@ -106,6 +155,11 @@ public class AbstractAccumulatorTest { | ... | @@ -106,6 +155,11 @@ public class AbstractAccumulatorTest { |
| 106 | batch += item.s; | 155 | batch += item.s; |
| 107 | } | 156 | } |
| 108 | } | 157 | } |
| 158 | + | ||
| 159 | + @Override | ||
| 160 | + public boolean isReady() { | ||
| 161 | + return ready; | ||
| 162 | + } | ||
| 109 | } | 163 | } |
| 110 | 164 | ||
| 111 | } | 165 | } | ... | ... |
-
Please register or login to post a comment