gravitino icon indicating copy to clipboard operation
gravitino copied to clipboard
Published 4 days ago • verified publisher icon apache

[4176]feat(iceberg) support multiple catalogs in the iceberg rest catalog server

Open theoryxu opened this issue 1 year ago • 9 comments

What changes were proposed in this pull request?

design doc: https://docs.google.com/document/d/1_XwsVHAzUjPCyOnJml69VkzhKI9veFvMo5OzGcKCEAk/edit

  1. Iceberg rest catalog clients could use gravitino iceberg catalogs through the iceberg-rest AuxiliaryService.
  2. Administers could enable or disable this feature by configuration.

Why are the changes needed?

The Gravitino catalog system could cooperate with the Iceberg catalog system more easily.

Fix: #4176

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

  1. Manually test
  2. Add UT

theoryxu avatar Jul 25 '24 08:07 theoryxu

@FANNG1 @jerryshao please help to review, Thanks.

theoryxu avatar Jul 25 '24 08:07 theoryxu

thanks for proposing the PR, some quick thought:

  1. how should we present the prefix, name or id? how other vendors like tabular organize it?
  2. we need something like catalogStore to retriew the catalog configurations, we'd better do some abstraction and make it pluggable not directly using Gravitino or entity store.
  3. a design doc is preferred.

FANNG1 avatar Jul 25 '24 10:07 FANNG1

thanks for proposing the PR, some quick thought:

  1. how should we present the prefix, name or id? how other vendors like tabular organize it?
  2. we need something like catalogStore to retriew the catalog configurations, we'd better do some abstraction and make it pluggable not directly using Gravitino or entity store.
  3. a design doc is preferred.

The design doc here, the above questions were responded to in it. We could discuss more in the doc. https://docs.google.com/document/d/1_XwsVHAzUjPCyOnJml69VkzhKI9veFvMo5OzGcKCEAk/edit

theoryxu avatar Jul 26 '24 02:07 theoryxu

could you add document to docs/iceberg-rest-service.md ?

FANNG1 avatar Aug 02 '24 02:08 FANNG1

could you add document to docs/iceberg-rest-service.md ?

ok, I have added the content to it.

theoryxu avatar Aug 02 '24 03:08 theoryxu

@FANNG1 could you help review again?

theoryxu avatar Aug 12 '24 02:08 theoryxu

@FANNG1 All checks have passed. Could you help review it again? Thanks

theoryxu avatar Aug 12 '24 06:08 theoryxu

LGTM except few comments, it's nearly ready to merge, @jerryshao do you have time to review again?

FANNG1 avatar Aug 12 '24 07:08 FANNG1

LGTM except few comments, it's nearly ready to merge, @jerryshao do you have time to review again?

All remaining flaws have been solved. Thanks for reviewing it.

theoryxu avatar Aug 12 '24 10:08 theoryxu

@theoryxu merged to main, thanks for your contribution.

FANNG1 avatar Aug 13 '24 10:08 FANNG1