jihaozh commented on a change in pull request #3842: [TE] detection - time
series provider loading cache
URL: https://github.com/apache/incubator-pinot/pull/3842#discussion_r258691823
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/DefaultDataProvider.java
##########
@@ -136,13 +150,17 @@ public DataFrame load(MetricSlice slice) {
}
@Override
- public void cacheTimeseries(Collection<MetricSlice> slices) {
- for (MetricSlice slice : slices){
- try {
- this.timeseriesCache.get(slice);
- } catch (Exception e){
- LOG.warn("cache time series for slice {} failed", slice.toString());
+ public Map<MetricSlice, DataFrame> fetchTimeseries(Collection<MetricSlice>
slices) {
+ try {
+ Map<MetricSlice, DataFrame> cacheResult =
DETECTION_TIME_SERIES_CACHE.getAll(slices);
+ Map<MetricSlice, DataFrame> timeseriesResult = new HashMap<>();
+ for (Map.Entry<MetricSlice, DataFrame> entry : cacheResult.entrySet()){
+ // make a copy of the result so that cache won't be contaminated by
client code
+ timeseriesResult.put(entry.getKey(), entry.getValue().copy());
Review comment:
yup. but even if we disable the cache, and re-fetch the data, it requires to
create a copy of time series anyways.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
