Skip to content
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

[#321] feat(core): add auxiliary service support in Graviton #406

Merged
merged 4 commits into from
Sep 20, 2023
Merged

[#321] feat(core): add auxiliary service support in Graviton #406

merged 4 commits into from
Sep 20, 2023

Conversation

FANNG1
Copy link
Contributor

@FANNG1 FANNG1 commented Sep 17, 2023

What changes were proposed in this pull request?

add a aux service to graviton to support Iceberg REST server

Why are the changes needed?

support Iceberg REST server

part: #321
will propose another PR to start Iceberg REST server.

Does this PR introduce any user-facing change?

add Iceberg REST server

How was this patch tested?

  1. UT
  2. start the Iceberg REST server in local env.

design docs

https://docs.google.com/document/d/1N_9NtaoHIeKQ36Q4fYZ9eGDotvjlB0-7lHKSoT76IZk/edit#heading=h.kbhoexq6d6ba

@FANNG1 FANNG1 marked this pull request as draft September 17, 2023 03:54
@github-actions
Copy link

github-actions bot commented Sep 17, 2023

Code Coverage Report

Overall Project 62.58% -0.76% 🟢
Files changed 50.95% 🔴

Module Coverage
server 89.37% 🟢
core 71.51% -2.67% 🔴
Files
Module File Coverage
server GravitonServer.java 57.96% 🟢
core MapUtils.java 89.29% -10.71% 🟢
CatalogManager.java 66.31% -0.33% 🔴
AuxiliaryServiceManager.java 58.56% -41.44% 🔴
IsolatedClassLoader.java 44.41% -17.82% 🔴
Config.java 37.8% 🟢
GravitonEnv.java 0% -18.24% 🔴

@xunliu
Copy link
Member

xunliu commented Sep 17, 2023

Hi @sandflee
When the Integration Test fails in the GitHub Action.

  • You can execute ./gradlew clean && ./gradlew build && ./gradle compileDistribution && ./gradle integrationTest in you local.
  • Or click Print logs when Graviton integration tests failure section in the GitHub Action, It will print logs/graviton-server.log in the GitHub Action to help you see the error log.

@xunliu
Copy link
Member

xunliu commented Sep 17, 2023

@sandflee Do you rebase your develop branch to the latest main branch?

Error: Exception in thread "main" java.lang.IllegalArgumentException: Invalid package path: catalogs/lakehouse/libs
	at com.datastrato.graviton.utils.IsolatedClassLoader.buildClassLoader(IsolatedClassLoader.java:82)
	at com.datastrato.graviton.aux.GravitonAuxServiceManager.registerAuxService(GravitonAuxServiceManager.java:56)
	at com.datastrato.graviton.aux.GravitonAuxServiceManager.lambda$registerAuxServices$2(GravitonAuxServiceManager.java:87)
	at java.util.Iterator.forEachRemaining(Iterator.java:116)
	at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
	at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:647)
	at com.datastrato.graviton.aux.GravitonAuxServiceManager.registerAuxServices(GravitonAuxServiceManager.java:85)
	at com.datastrato.graviton.aux.GravitonAuxServiceManager.serviceInit(GravitonAuxServiceManager.java:113)
	at com.datastrato.graviton.GravitonEnv.initialize(GravitonEnv.java:84)
	at com.datastrato.graviton.server.GravitonServer.initialize(GravitonServer.java:50)
	at com.datastrato.graviton.server.GravitonServer.main(GravitonServer.java:93)

@yuqi1129
Copy link
Contributor

add a aux service to graviton to support Iceberg REST server

Can you provide a more detailed description of this PR?

@FANNG1
Copy link
Contributor Author

FANNG1 commented Sep 19, 2023

add a aux service to graviton to support Iceberg REST server

Can you provide a more detailed description of this PR?

add design docs~

We do not have privileges to access this document.

@FANNG1 FANNG1 marked this pull request as ready for review September 19, 2023 07:49
@jerryshao jerryshao changed the title [#321] feat(core): add aux service to graviton server [#321] feat(core): add auxiliary service support in Graviton Sep 20, 2023

String gravitonHome = System.getenv("GRAVITON_HOME");
if (!path.isAbsolute() && gravitonHome != null) {
Path newPath = Paths.get(gravitonHome, pathString);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you configure the "classpath" config, is it a relative path or a absolute path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it supports both relative path and absolute path, if it's a relative path, it relative to gravitonHome

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if tested in MiniGraviton, catalogs jars is placed in catalog-xx/build/libs/, will try to get aux test classpath in auxServiceTestClassPaths if in test mode

// in product mode, catalogs jars is placed in catalogs/xx/libs/
private static final Map<String, String> auxServiceTestClassPaths =
ImmutableMap.of(
"GravitonIcebergREST", Paths.get("catalog-lakehouse", "build", "libs").toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can inject the configuration when running mini graviton mode, I don't think it is a good idea to hardcode here.

@jerryshao jerryshao merged commit 0212b05 into apache:main Sep 20, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants