[#198] test: Add Catalog-hive e2e integration test #308

xunliu · 2023-09-01T05:36:27Z

What changes were proposed in this pull request?

Add Catalog-hive integration test in the integration-test module
Run integration test in the GitHub Action AMD64 architecture (support ARM64 future in the next PR)
Add some documents
Add build docker image label in the PR, It enables/disable rebuilding the docker image in the Pull Request CI step.

Why are the changes needed?

Launching the Graviton Server process in the local host or GitHub CI environment.
Through Graviton client connects the Graviton Server and hive docker container to test Catalog-hive.

Fix: #198

Does this PR introduce any user-facing change?

N/A

How was this patch tested?

Test in the GitHub Action

github-actions · 2023-09-01T05:46:28Z

Code Coverage Report

Overall Project	62.99% `-0.25%`	🟢
Files changed	18.18%	🔴

Module	Coverage
core	71.35% `-0.74%`	🔴
catalog-hive	58.45% `-0.07%`	🟢

Files

Module	File	Coverage
core	CatalogManager.java	62.26% `-6.1%`	🔴
	IsolatedClassLoader.java	54.04%	🟢
	CatalogOperationDispatcher.java	17.71% `-2.24%`	🔴
catalog-hive	HiveClientPool.java	53.46% `-1.89%`	🟢

.github/workflows/integration-test.yml

jerryshao · 2023-09-01T07:11:32Z

.github/workflows/integration-test.yml

+env:
+  HIVE2_IMAGE_NAME: datastrato/hive2
+  HIVE2_IMAGE_VERSION: 0.1.0
+  HIVE2_IMAGE_LATEST: latest


What's usage of "HIVE_IMAGE_VERSION" and "HIVE_IMAGE_LATEST"

Many places needs to use it.

Build the hive2 Docker image for AMD64

Run AMD64 container

Clean build docker image envorionment

My question is that "HIVE2_IMAGE_VERSION" seemed not used in this script. Also seems "HIVE_IMAGE_VERSION" and "HIVE_IMAGE_LATEST" they all point to the version things, do we need to unify them?

Just a reminder that with ASF projects you can only publish releases to Docker and can't refer to the latest. If we go with publishing the latest we'll need to change that when we enter the ASF incubator.

Just a reminder that with ASF projects you can only publish releases to Docker and can't refer to the latest. If we go with publishing the latest we'll need to change that when we enter the ASF incubator.
@justinmclean Thank you for your reminder.

I modify this code.

.github/workflows/integration-test.yml

jerryshao · 2023-09-01T07:20:55Z

catalog-hive/src/main/java/com/datastrato/graviton/catalog/hive/HiveCatalogOperations.java

+    // The specify Hadoop user name that will be used when accessing Hadoop
+    // Distributed File System (HDFS).
+    if (conf.containsKey(HiveCatalogConfig.HADOOP_USER_NAME.getKey())) {
+      System.setProperty("HADOOP_USER_NAME", conf.get(HiveCatalogConfig.HADOOP_USER_NAME.getKey()));


Why do we need this?

The specific Hadoop user name that will be used when accessing Hadoop Distributed File System (HDFS).
I think using HADOOP_USER_NAME environment is not good. Because users need to use HiveCatalogOperations.java operation in many different Hive Clusters.
Wait add Graviton User Account System to manage users and groups in the future.

Is it possible that we remove this configuration and use some other ways to bypass this issue? I don't think using configuration here is a good idea?

Is it possible that we set this property in the test?

Modify to set HADOOP_USER_NAME environment in the integration-test module.

docs/integration-test.md

integration-test/build.gradle.kts

integration-test/src/test/java/com/datastrato/graviton/integration/e2e/CatalogHiveIT.java

integration-test/src/test/java/com/datastrato/graviton/integration/e2e/MetalakeIT.java

xunliu · 2023-09-03T15:55:51Z

integration-test/src/test/java/com/datastrato/graviton/integration/e2e/CatalogHiveIT.java

+  // TODO (xun) enable this test waiting for fixed [#316] [Bug report] alterSchema throw
+  // NoSuchSchemaException
+  //  @Order(3)
+  //  @Test
+  public void testAlterSchema() throws TException, InterruptedException {
+    NameIdentifier ident = NameIdentifier.of(metalakeName, catalogName, schemaName);
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put("key1", "val1");
+    properties.put("key2", "val2");
+    String comment = "comment";
+
+    GravitonMetaLake metalake = client.loadMetalake(NameIdentifier.of(metalakeName));
+    Catalog catalog = metalake.loadCatalog(NameIdentifier.of(metalakeName, catalogName));
+    catalog
+        .asSchemas()
+        .alterSchema(
+            ident,
+            SchemaChange.removeProperty("key1"),
+            SchemaChange.setProperty("key2", "val2-alter"));
+
+    NameIdentifier[] nameIdentifiers = catalog.asSchemas().listSchemas(ident.namespace());
+
+    Map<String, String> properties2 = catalog.asSchemas().loadSchema(ident).properties();
+    Assertions.assertFalse(properties2.containsKey("key1"));
+    Assertions.assertEquals("val2-alter", properties2.get("key2"));
+
+    Database database = hiveClientPool.run(client -> client.getDatabase(schemaName));
+    Map<String, String> properties3 = database.getParameters();
+    Assertions.assertFalse(properties3.containsKey("key1"));
+    Assertions.assertEquals("val2-alter", properties3.get("key2"));
+  }


testAlterSchema() throws an exception.
I created #316 [Bug report] alterSchema throw to track this bug.
Temporarily disable this test case.

We can debug it together tomorrow;

.github/workflows/integration-test.yml

docs/how-to-build.md

docs/integration-test.md

integration-test/build.gradle.kts

yuqi1129 · 2023-09-04T08:34:54Z

integration-test/src/test/java/com/datastrato/graviton/integration/e2e/CatalogHiveIT.java

+  // TODO (xun) enable this test waiting for fixed [#316] [Bug report] alterSchema throw
+  // NoSuchSchemaException
+  //  @Order(3)
+  //  @Test
+  public void testAlterSchema() throws TException, InterruptedException {
+    NameIdentifier ident = NameIdentifier.of(metalakeName, catalogName, schemaName);
+    Map<String, String> properties = Maps.newHashMap();
+    properties.put("key1", "val1");
+    properties.put("key2", "val2");
+    String comment = "comment";
+
+    GravitonMetaLake metalake = client.loadMetalake(NameIdentifier.of(metalakeName));
+    Catalog catalog = metalake.loadCatalog(NameIdentifier.of(metalakeName, catalogName));
+    catalog
+        .asSchemas()
+        .alterSchema(
+            ident,
+            SchemaChange.removeProperty("key1"),
+            SchemaChange.setProperty("key2", "val2-alter"));
+
+    NameIdentifier[] nameIdentifiers = catalog.asSchemas().listSchemas(ident.namespace());
+
+    Map<String, String> properties2 = catalog.asSchemas().loadSchema(ident).properties();
+    Assertions.assertFalse(properties2.containsKey("key1"));
+    Assertions.assertEquals("val2-alter", properties2.get("key2"));
+
+    Database database = hiveClientPool.run(client -> client.getDatabase(schemaName));
+    Map<String, String> properties3 = database.getParameters();
+    Assertions.assertFalse(properties3.containsKey("key1"));
+    Assertions.assertEquals("val2-alter", properties3.get("key2"));
+  }


We can debug it together tomorrow;

yuqi1129 · 2023-09-04T08:57:12Z

.github/workflows/integration-test.yml

+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v1
+
+      - name: Build the hive2 Docker image for AMD64


Most of the Docker Official Images on Docker Hub provide a variety of architecturesopen_in_new. For example, the busybox image supports amd64, arm32v5, arm32v6, arm32v7, arm64v8, i386, ppc64le, and s390x. When running this image on an x86_64 / amd64 machine, the amd64 variant is pulled and run.

https://docs.docker.com/build/building/multi-platform/#:~:text=Most%20of%20the%20Docker%20Official,variant%20is%20pulled%20and%20run.

core/src/main/java/com/datastrato/graviton/catalog/CatalogManager.java

xunliu · 2023-09-05T08:39:30Z

@jerryshao I fixed the problem in the comments. Please help me review PR, Thanks.

### What changes were proposed in this pull request? 1. Add Catalog-hive integration test in the `integration-test` module 2. Run integration test in the GitHub Action AMD64 architecture (support ARM64 future in the next PR) 3. Add some documents 4. Add `build docker image` label in the PR, It enables/disable rebuilding the docker image in the Pull Request CI step. ### Why are the changes needed? Launching the Graviton Server process in the local host or GitHub CI environment. Through Graviton client connects the Graviton Server and hive docker container to test Catalog-hive. Fix: #198 ### Does this PR introduce _any_ user-facing change? N/A ### How was this patch tested? Test in the GitHub Action

Catalog-hive e2e integration test

da87b55

xunliu added the build docker image label Sep 1, 2023

xunliu requested a review from jerryshao September 1, 2023 05:36

jerryshao added need backport Issues that need to backport to another branch branch-0.1 labels Sep 1, 2023

xunliu mentioned this pull request Sep 1, 2023

[Subtask] Simplify hive matestore configuration #196

Closed

jerryshao reviewed Sep 1, 2023

View reviewed changes

xunliu added 5 commits September 3, 2023 20:12

fix

a2dbed7

Remove log

dfb5151

Fixed

bb3365d

Removed log

7dbd22d

Java checked

b60f530

xunliu commented Sep 3, 2023

View reviewed changes

yuqi1129 reviewed Sep 4, 2023

View reviewed changes

xunliu added 2 commits September 5, 2023 13:22

fix

922cc96

fixed

bcbe6c9

xunliu added 2 commits September 5, 2023 17:05

HIVE2_IMAGE_TAG_NAME

3095733

Set docker tag name

8431104

jerryshao closed this Sep 5, 2023

jerryshao reopened this Sep 5, 2023

jerryshao approved these changes Sep 5, 2023

View reviewed changes

jerryshao merged commit 5b26878 into apache:main Sep 5, 2023

xunliu deleted the issue-198 branch October 9, 2023 02:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#198] test: Add Catalog-hive e2e integration test #308

[#198] test: Add Catalog-hive e2e integration test #308

xunliu commented Sep 1, 2023 •

edited by jerryshao

Loading

github-actions bot commented Sep 1, 2023 •

edited

Loading

jerryshao Sep 1, 2023

xunliu Sep 3, 2023 •

edited

Loading

jerryshao Sep 4, 2023

justinmclean Sep 4, 2023

xunliu Sep 5, 2023

jerryshao Sep 1, 2023

xunliu Sep 3, 2023

jerryshao Sep 4, 2023

jerryshao Sep 4, 2023

xunliu Sep 5, 2023

xunliu Sep 3, 2023

yuqi1129 Sep 4, 2023

yuqi1129 Sep 4, 2023

yuqi1129 Sep 4, 2023

xunliu Sep 5, 2023

xunliu commented Sep 5, 2023

[#198] test: Add Catalog-hive e2e integration test #308

[#198] test: Add Catalog-hive e2e integration test #308

Conversation

xunliu commented Sep 1, 2023 • edited by jerryshao Loading

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

github-actions bot commented Sep 1, 2023 • edited Loading

Code Coverage Report

Choose a reason for hiding this comment

xunliu Sep 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xunliu commented Sep 5, 2023

xunliu commented Sep 1, 2023 •

edited by jerryshao

Loading

github-actions bot commented Sep 1, 2023 •

edited

Loading

xunliu Sep 3, 2023 •

edited

Loading