search

datastrato / gravitino

World's most powerful open data catalog for building a high-performance, geo-distributed and federated metadata lake.
https://datastrato.ai/docs/
Apache License 2.0
609 stars 193 forks source link

[#1187] feat(lakehouse-iceberg): the basic framework of storing Iceberg metrics #1164

Closed FANNG1 closed 5 months ago

FANNG1 commented 6 months ago

What changes were proposed in this pull request?

  1. introduce MetricsManager to manage storages
  2. introduce IcebergMetricsFormatter to format metrics, support JSON for now.

Why are the changes needed?

Fix: #1187

Does this PR introduce any user-facing change?

no

How was this patch tested?

  1. local env
jerryshao commented 6 months ago

What's the purpose of writing metrics to FS, not db?

FANNG1 commented 6 months ago

What's the purpose of writing metrics to FS, not db?

Analyzing the metrics with Spark or Python is easy if writing to FS.

FANNG1 commented 6 months ago

remove the specified storage implement, leaving the basic framework.

FANNG1 commented 6 months ago

@Clearvive @jerryshao , could you help to review this pr?

qqqttt123 commented 6 months ago

Do we need a metrics storage for the Gravitino server?

FANNG1 commented 6 months ago

Do we need a metrics storage for the Gravitino server?

no, it's used to store Iceberg query or commit metrics not server metrics.

FANNG1 commented 6 months ago

@Clearvive @yuqi1129 , are comments addressed, please review again. thx.

Clearvive commented 6 months ago

LGTM except the few comments

FANNG1 commented 5 months ago

@jerryshao , all comments are addressed, please help to review again.

FANNG1 commented 5 months ago

planing to do:

  1. remove async mode configuration
  2. add queue capacity configutation
  3. use service loader to load specified metrics storage.
FANNG1 commented 5 months ago

planing to do:

  1. remove async mode configuration
  2. add queue capacity configutation
  3. use service loader to load specified metrics storage.

supported in the lastest pr , cc @jerryshao

jerryshao commented 5 months ago

I think there's no need to use service loader to make it pluggable, using reflection should be enough.

FANNG1 commented 5 months ago

I think there's no need to use service loader to make it pluggable, using reflection should be enough.

replace service loader with reflection.