Hi Yuan,
> how can we tell the real “identifier” and “type” options in effect to users?
If I understand correctly these are specified in DDL table options by users.
For example:
CREATE TABLE DimTable (…) WITH (
...
“cache.identifier” = “guava”,
“cache.type” = “LRU”
);
> Does MetricNames.java contain all metric names?
I don’t think there's a rule that all metric names should be in MetricNames
class, but it would be great to aggregate these constants into a unified place.
Cheers,
Qingsheng
> On Mar 8, 2022, at 10:22, zst...@163.com wrote:
>
> Hi Qingsheng Ren,
> Thanks for your feedback.
>
>
>> 1. It looks like “identifier” and “type” are options of cache instead of
>> metrics. I think they are finalized once the cache is created so maybe it’s
>> not quite helpful to report them to the metric system.
>
>
> Maybe it's not quite appropriate to report them to the metric system, but how
> can we tell the real “identifier” and “type” options in effect to users?
>
>
>
>
>> 2. Some metrics can be aggregated simply in metric systems, like loadCount =
>> loadSuccessCount + loadExceptionCount, so maybe we can just keep fundamental
>> metrics (like loadSuccessCount and loadExceptionCount) to avoid redundancy.
>
>
> I agree with you. I have removed these redundant metrics.
>
>
>> 3. About the interface of CacheMetricGroup I think it would be easier for
>> cache implementers to use if we expose wrapped function instead of let users
>> provide gauges directly.
>
>
> Thanks for your advice, they are helpful and I have adjusted it. I have a
> question about it. Does MetricNames.java
> <https://github.com/apache/flink/blob/master/flink-runtime%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fflink%2Fruntime%2Fmetrics%2FMetricNames.java>
> contain all metric names? Should I put the cache metric names here?
>
>
>
>
>
>
> Best regards,
> Yuan
>
>
>
>
>
> At 2022-03-07 16:55:18, "Qingsheng Ren" <renqs...@gmail.com> wrote:
>> Hi Yuan,
>>
>> Thanks for raising this discussion! I believe this will be very helpful for
>> lookup table developers, and standardizing metrics would be essential for
>> users to tuning their systems.
>>
>> Here’s some thoughts in my mind:
>>
>> 1. It looks like “identifier” and “type” are options of cache instead of
>> metrics. I think they are finalized once the cache is created so maybe it’s
>> not quite helpful to report them to the metric system.
>>
>> 2. Some metrics can be aggregated simply in metric systems, like loadCount =
>> loadSuccessCount + loadExceptionCount, so maybe we can just keep fundamental
>> metrics (like loadSuccessCount and loadExceptionCount) to avoid redundancy.
>>
>> 3. About the interface of CacheMetricGroup I think it would be easier for
>> cache implementers to use if we expose wrapped function instead of let users
>> provide gauges directly. For example:
>>
>> public interface CacheMetricGroup extends MetricGroup {
>> // Mark a cache hit
>> public void markCacheHit();
>> // Mark a cache miss
>> public void recordCacheMiss();
>> ...
>> }
>>
>> You can check SourceReaderMetricGroup[1] and its implementation[2] as a
>> reference.
>>
>> Hope these would be helpful!
>>
>> Best regards,
>>
>> Qingsheng Ren
>>
>> [1]
>> https://github.com/apache/flink/blob/master/flink-metrics/flink-metrics-core/src/main/java/org/apache/flink/metrics/groups/SourceReaderMetricGroup.java
>> [2]
>> https://github.com/apache/flink/blob/master/flink-runtime/src/main/java/org/apache/flink/runtime/metrics/groups/InternalSourceReaderMetricGroup.java
>>
>>
>>> On Mar 7, 2022, at 16:00, zst...@163.com wrote:
>>>
>>> Hi devs,
>>>
>>>
>>> I would like to propose a discussion thread about abstraction of Cache
>>> LookupFunction with metrics for cache in connectors to make cache out of
>>> box for connector developers. There are multiple LookupFunction
>>> implementations in individual connectors [1][2][3][4] so far.
>>> At the same time, users can monitor cache in LookupFunction by adding
>>> uniform cache metrics to optimize tasks or troubleshoot.
>>>
>>>
>>> I have posted an issue about this, see
>>> <https://issues.apache.org/jira/browse/FLINK-25409>, and made a brief
>>> design
>>> <https://docs.google.com/document/d/1L2eo7VABZBdRxoRP_wPvVwuvTZOV9qrN9gEQxjhSJOc/edit?usp=sharing>.
>>>
>>>
>>> Looking forward to your feedback, thanks.
>>>
>>>
>>> Best regards,
>>> Yuan
>>>
>>>
>>>
>>>
>>> [1]
>>> https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-jdbc/src/main/java/org/apache/flink/connector/jdbc/table/JdbcRowDataLookupFunction.java
>>> [2]
>>> https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/connectors/hive/FileSystemLookupFunction.java
>>> [3]
>>> https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-hbase-base/src/main/java/org/apache/flink/connector/hbase/source/HBaseRowDataLookupFunction.java
>>> [4]
>>> https://github.com/apache/flink/blob/master/flink-connectors/flink-connector-hbase-2.2/src/main/java/org/apache/flink/connector/hbase2/source/HBaseRowDataAsyncLookupFunction.java
