-
Notifications
You must be signed in to change notification settings - Fork 331
SAMZA-1733 New metric type Listgauge #541
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
5ae1df9
5a3ef7b
75ef5d2
1efed5a
d2922c5
1f08eae
6f19ba0
4f08cd1
bdc85d4
5c1ed0a
c1884e4
c7821ba
9ba8e5b
fd77c53
cc4d881
863d850
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| /* | ||
| * 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 org.apache.samza.metrics; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.util.Queue; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.ScheduledExecutorService; | ||
| import java.util.concurrent.TimeUnit; | ||
| import org.apache.samza.util.TimestampedValue; | ||
|
|
||
|
|
||
| /** | ||
| * Provides an eviction policy that evicts entries from the elements if | ||
| * a.) There are more elements in the elements than the specified maxNumberOfItems (removal in FIFO order), or | ||
| * b.) There are elements which have timestamps which are stale as compared to currentTime (the staleness bound is | ||
| * specified as maxStaleness). | ||
| * | ||
| * This naive implementation uses a periodic thread with a configurable period. | ||
| */ | ||
| public class DefaultListGaugeEvictionPolicy<T> { | ||
|
|
||
| private final Queue<TimestampedValue<T>> elements; | ||
| private final int nItems; | ||
| private final Duration durationThreshold; | ||
| private final ScheduledExecutorService scheduledExecutorService; | ||
|
|
||
| public DefaultListGaugeEvictionPolicy(Queue<TimestampedValue<T>> elements, int maxNumberOfItems, | ||
| Duration maxStaleness, Duration period) { | ||
| this.elements = elements; | ||
| this.nItems = maxNumberOfItems; | ||
| this.durationThreshold = maxStaleness; | ||
| this.scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); | ||
| this.scheduledExecutorService.schedule(new EvictionRunnable(), period.toMillis(), TimeUnit.MILLISECONDS); | ||
| } | ||
|
|
||
| public void elementAddedCallback() { | ||
|
|
||
| // need to synchronize here because this thread could be concurrent with the runnable thread and can | ||
| // cause two vals to be removed (wrong eviction) even if a threadsafe queue was used. | ||
| synchronized (this.elements) { | ||
| int numToEvict = this.elements.size() - nItems; | ||
|
|
||
| while (numToEvict > 0) { | ||
| this.elements.poll(); // remove head | ||
| numToEvict--; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private class EvictionRunnable implements Runnable { | ||
|
|
||
| @Override | ||
| public void run() { | ||
| Instant currentTimestamp = Instant.now(); | ||
|
|
||
| synchronized (elements) { | ||
| TimestampedValue<T> valueInfo = elements.peek(); | ||
|
|
||
| // continue remove-head if currenttimestamp - head-element's timestamp > durationThreshold | ||
| while (valueInfo != null | ||
| && currentTimestamp.toEpochMilli() - valueInfo.getTimestamp() > durationThreshold.toMillis()) { | ||
| elements.poll(); | ||
| valueInfo = elements.peek(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| /* | ||
| * 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 org.apache.samza.metrics; | ||
|
|
||
| import java.time.Duration; | ||
| import java.time.Instant; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.Queue; | ||
| import java.util.concurrent.ConcurrentLinkedQueue; | ||
| import java.util.stream.Collectors; | ||
| import org.apache.samza.util.TimestampedValue; | ||
|
|
||
|
|
||
| /** | ||
| * A {@link ListGauge} is a {@link org.apache.samza.metrics.Metric} that buffers multiple instances of a type T in a list. | ||
| * {@link ListGauge}s are useful for maintaining, recording, or collecting values over time. | ||
| * For example, a set of specific logging-events (e.g., errors). | ||
| * | ||
| * Eviction from list is either done by consuming-code using the remove APIs or by specifying an eviction policy | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nuke L:36 and update it so that it's inline with the current implementation |
||
| * at creation time. | ||
| * | ||
| * All public methods are thread-safe. | ||
| * | ||
| */ | ||
| public class ListGauge<T> implements Metric { | ||
| private final String name; | ||
| private final Queue<TimestampedValue<T>> elements; | ||
| private final DefaultListGaugeEvictionPolicy<T> listGaugeEvictionPolicy; | ||
|
|
||
| private final static int DEFAULT_MAX_NITEMS = 1000; | ||
| private final static Duration DEFAULT_MAX_STALENESS = Duration.ofMinutes(60); | ||
| private final static Duration DEFAULT_EVICTION_CHECK_PERIOD = Duration.ofMinutes(1); | ||
|
|
||
| /** | ||
| * Create a new {@link ListGauge} that auto evicts based on the given maxNumberOfItems, maxStaleness, and period parameters. | ||
| * | ||
| * @param name Name to be assigned | ||
| * @param maxNumberOfItems The max number of items that can remain in the listgauge | ||
| * @param maxStaleness The max staleness of items permitted in the listgauge | ||
| * @param period The periodicity with which the listGauge would be checked for stale values. | ||
| */ | ||
| public ListGauge(String name, int maxNumberOfItems, Duration maxStaleness, Duration period) { | ||
| this.name = name; | ||
| this.elements = new ConcurrentLinkedQueue<TimestampedValue<T>>(); | ||
| this.listGaugeEvictionPolicy = | ||
| new DefaultListGaugeEvictionPolicy<T>(this.elements, maxNumberOfItems, maxStaleness, period); | ||
| } | ||
|
|
||
| /** | ||
| * Create a new {@link ListGauge} that auto evicts upto a max of 100 items and a max-staleness of 60 minutes. | ||
| * @param name Name to be assigned | ||
| */ | ||
| public ListGauge(String name) { | ||
| this(name, DEFAULT_MAX_NITEMS, DEFAULT_MAX_STALENESS, DEFAULT_EVICTION_CHECK_PERIOD); | ||
| } | ||
|
|
||
| /** | ||
| * Get the name assigned to this {@link ListGauge} | ||
| * @return the assigned name | ||
| */ | ||
| public String getName() { | ||
| return this.name; | ||
| } | ||
|
|
||
| /** | ||
| * Get the Collection of Gauge values currently in the list, used when serializing this Gauge. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| * @return the collection of gauge values | ||
| */ | ||
| public Collection<T> getValues() { | ||
| return Collections.unmodifiableList(this.elements.stream().map(x -> x.getValue()).collect(Collectors.toList())); | ||
| } | ||
|
|
||
| /** | ||
| * Add a value to the list. | ||
| * (Timestamp assigned to this value is the current timestamp.) | ||
| * @param value The Gauge value to be added | ||
| */ | ||
| public void add(T value) { | ||
| this.elements.add(new TimestampedValue<T>(value, Instant.now().toEpochMilli())); | ||
|
|
||
| // notify the policy object for performing any eviction that may be needed. | ||
| this.listGaugeEvictionPolicy.elementAddedCallback(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now that there is a need for only 2 types of evictions (size, time), I wonder if we could just do evictions If you still think we need an interface for a pluggable eviction policy, it could be something on the lines of: instead of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done, purged the interface and just used the default class impl |
||
| } | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| @Override | ||
| public void visit(MetricsVisitor visitor) { | ||
| visitor.listGauge(this); | ||
| } | ||
|
|
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| /* | ||
| * 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 org.apache.samza.metrics; | ||
|
|
||
| import java.time.Duration; | ||
| import java.util.Collection; | ||
| import java.util.Iterator; | ||
| import org.junit.Assert; | ||
| import org.junit.Test; | ||
|
|
||
|
|
||
| /** | ||
| * Class to encapsulate test-cases for {@link org.apache.samza.metrics.ListGauge} | ||
| */ | ||
| public class TestListGauge { | ||
|
|
||
| private final static Duration THREAD_TEST_TIMEOUT = Duration.ofSeconds(10); | ||
|
|
||
| private <T> ListGauge<T> getListGaugeForTest() { | ||
| return new ListGauge<T>("sampleListGauge", 10, Duration.ofSeconds(60), Duration.ofSeconds(60)); | ||
| } | ||
|
|
||
| @Test | ||
| public void basicTest() { | ||
| ListGauge<String> listGauge = getListGaugeForTest(); | ||
| listGauge.add("sampleValue"); | ||
| Assert.assertEquals("Names should be the same", listGauge.getName(), "sampleListGauge"); | ||
| Assert.assertEquals("List sizes should match", listGauge.getValues().size(), 1); | ||
| Assert.assertEquals("ListGauge should contain sampleGauge", listGauge.getValues().contains("sampleValue"), true); | ||
| } | ||
|
|
||
| @Test | ||
| public void testSizeEnforcement() { | ||
| ListGauge listGauge = getListGaugeForTest(); | ||
| for (int i = 15; i > 0; i--) { | ||
| listGauge.add("v" + i); | ||
| } | ||
| Assert.assertEquals("List sizes should be as configured at creation time", listGauge.getValues().size(), 10); | ||
|
|
||
| int valueIndex = 10; | ||
| Collection<String> currentList = listGauge.getValues(); | ||
| Iterator iterator = currentList.iterator(); | ||
| while (iterator.hasNext()) { | ||
| String gaugeValue = (String) iterator.next(); | ||
| Assert.assertTrue(gaugeValue.equals("v" + valueIndex)); | ||
| valueIndex--; | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testThreadSafety() throws InterruptedException { | ||
| ListGauge<Integer> listGauge = getListGaugeForTest(); | ||
|
|
||
| Thread thread1 = new Thread(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| for (int i = 1; i <= 100; i++) { | ||
| listGauge.add(i); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| Thread thread2 = new Thread(new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| for (int i = 1; i <= 100; i++) { | ||
| listGauge.add(i); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| thread1.start(); | ||
| thread2.start(); | ||
|
|
||
| thread1.join(THREAD_TEST_TIMEOUT.toMillis()); | ||
| thread2.join(THREAD_TEST_TIMEOUT.toMillis()); | ||
|
|
||
| Assert.assertTrue("ListGauge should have the last 10 values", listGauge.getValues().size() == 10); | ||
| for (Integer gaugeValue : listGauge.getValues()) { | ||
| Assert.assertTrue("Values should have the last 10 range", gaugeValue <= 100 && gaugeValue > 90); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -62,7 +62,7 @@ public void testTimerWithDifferentWindowSize() { | |
| assertTrue(snapshot.getValues().containsAll(Arrays.asList(1L, 2L, 3L))); | ||
| assertEquals(3, snapshot.getValues().size()); | ||
|
|
||
| // The time is 500 for update(4L) because getSnapshot calls clock once + 3 | ||
| // The time is 500 for update(4L) because getValues calls clock once + 3 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. revert this? |
||
| // updates that call clock 3 times | ||
| timer.update(4L); | ||
| Snapshot snapshot2 = timer.getSnapshot(); | ||
|
|
||
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.
minor: Do you need the fully qualified name "{@link org.apache.samza.metrics.Metric}" here? it seems like you should be able to use {@link Metric}