xushiyan commented on code in PR #5064: URL: https://github.com/apache/hudi/pull/5064#discussion_r942017639
########## hudi-metaserver/src/main/thrift/gen-java/org/apache/hudi/metaserver/thrift/AlreadyExistException.java: ########## @@ -0,0 +1,377 @@ +/** + * Autogenerated by Thrift Compiler (0.13.0) Review Comment: is it possible to not version control this kind of generated code? we only need to have `hudi-metaserver.thrift`. It should be part of the compilation and packaging step before server is started. The code gen can also be part of `hudi-metaserver-bundle` packaging step. ########## hudi-metaserver/src/main/java/org/apache/hudi/metaserver/store/RelationDBBasedStore.java: ########## @@ -0,0 +1,244 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metaserver.store; + +import org.apache.hudi.metaserver.store.bean.InstantBean; +import org.apache.hudi.metaserver.store.bean.TableBean; +import org.apache.hudi.metaserver.store.jdbc.WrapperDao; +import org.apache.hudi.metaserver.thrift.MetaStoreException; +import org.apache.hudi.metaserver.thrift.NoSuchObjectException; +import org.apache.hudi.metaserver.thrift.Table; +import org.apache.hudi.metaserver.thrift.THoodieInstant; +import org.apache.hudi.metaserver.thrift.TState; + +import java.io.Serializable; +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.Arrays; +import java.util.Date; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; + +/** + * Metadata store based on relation database. + */ +public class RelationDBBasedStore implements MetadataStore, Serializable { Review Comment: `Relation` -> `Relational`. ########## hudi-metaserver/src/main/java/org/apache/hudi/metaserver/service/SnapshotService.java: ########## @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metaserver.service; + +import org.apache.hudi.metaserver.store.MetadataStore; + +import java.io.Serializable; +import java.nio.ByteBuffer; + +/** + * Handle all snapshot / files related requests. + */ +public class SnapshotService implements Serializable { + private MetadataStore store; + + public SnapshotService(MetadataStore metadataStore) { + this.store = metadataStore; + } + + // TODO: support snapshot + public ByteBuffer listFilesInPartition(String db, String tb, String partition, String timestamp) { + return null; Review Comment: not sure why you define return value as ByteBuffer. The API name implies return value should list or array. Also please avoid returning null from public APIs. Use either Option<> or empty collection/array ########## hudi-metaserver/src/test/java/org/apache/hudi/metaserver/store/TestRelationDBBasedStore.java: ########## @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metaserver.store; + +import org.apache.hudi.metaserver.HoodieMetaServer; +import org.apache.hudi.metaserver.thrift.NoSuchObjectException; +import org.apache.hudi.metaserver.thrift.TAction; +import org.apache.hudi.metaserver.thrift.THoodieInstant; +import org.apache.hudi.metaserver.thrift.TState; +import org.apache.hudi.metaserver.thrift.MetaStoreException; +import org.apache.hudi.metaserver.thrift.Table; +import org.apache.thrift.TException; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.List; + +/** + * Unit tests on metadata store base on relation database of hoodie meta server. + */ +public class TestRelationDBBasedStore { + + private MetadataStore store; + private final String db = "test_db"; + private final String tb = "test_tb"; + + @Test + public void testAPIs() throws TException { + HoodieMetaServer.getEmbeddedMetaServer(); + store = HoodieMetaServer.getMetadataStore(); + testTableRelatedAPIs(); + testTimelineRelatedAPIs(); + } + + private void testTableRelatedAPIs() throws MetaStoreException, NoSuchObjectException { + Assertions.assertTrue(store.createDatabase(db)); Review Comment: let's avoid the style of `Assertions.assertTrue`. it increases code verbosity. we should use static import instead. ########## hudi-metaserver/src/main/java/org/apache/hudi/metaserver/client/RetryingHoodieMetaServerClient.java: ########## @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metaserver.client; + +import org.apache.hudi.common.config.HoodieMetaServerConfig; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; + +import java.io.Serializable; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Proxy; +import java.lang.reflect.UndeclaredThrowableException; + +/** + * AOP for meta server client. + */ +public class RetryingHoodieMetaServerClient implements InvocationHandler, Serializable { Review Comment: we should prefix class with `Hoodie`. No need to state `Retrying` in the classname given it's a client; isn't that common for a client to be able to retry? ########## hudi-metaserver/src/main/java/org/apache/hudi/metaserver/client/HoodieMetaServerClientImp.java: ########## @@ -0,0 +1,194 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hudi.metaserver.client; + +import org.apache.hadoop.fs.FileStatus; +import org.apache.hudi.common.config.HoodieMetaServerConfig; +import org.apache.hudi.common.table.timeline.HoodieInstant; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.exception.HoodieException; +import org.apache.hudi.metaserver.HoodieMetaServer; +import org.apache.hudi.metaserver.thrift.Table; +import org.apache.hudi.metaserver.thrift.ThriftHoodieMetaServer; +import org.apache.hudi.metaserver.util.EntryConvertor; +import org.apache.log4j.LogManager; +import org.apache.log4j.Logger; +import org.apache.thrift.TException; +import org.apache.thrift.protocol.TBinaryProtocol; +import org.apache.thrift.transport.TSocket; +import org.apache.thrift.transport.TTransport; +import org.apache.thrift.transport.TTransportException; + +import java.io.Serializable; +import java.net.URI; +import java.net.URISyntaxException; +import java.nio.ByteBuffer; +import java.util.List; +import java.util.function.Supplier; +import java.util.stream.Collectors; + +/** + * HoodieMetaServerClientImp based on thrift. + */ +public class HoodieMetaServerClientImp implements HoodieMetaServerClient, Serializable { + + private static final Logger LOG = LogManager.getLogger(HoodieMetaServerClientImp.class); + private final HoodieMetaServerConfig config; + private final int retryLimit; + private final int retryDelaySeconds; + private boolean isConnected; + private boolean isLocal; + private ThriftHoodieMetaServer.Iface client; + private TTransport transport; + + public HoodieMetaServerClientImp(HoodieMetaServerConfig config) { + this.config = config; + this.retryLimit = config.getConnectionRetryLimit(); + this.retryDelaySeconds = config.getConnectionRetryDelay(); + String uri = config.getMetaServerUris(); + if (isLocalEmbeddedMetaServer(uri)) { + this.client = HoodieMetaServer.getEmbeddedMetaServer(); + this.isConnected = true; + this.isLocal = true; + } else { + open(); + } + } + + private void open() { + String uri = config.getMetaServerUris(); + TTransportException exception = null; + for (int i = 0; !isConnected && i < retryLimit; i++) { + try { + URI msUri = new URI(uri); + this.transport = new TSocket(msUri.getHost(), msUri.getPort()); + this.client = new ThriftHoodieMetaServer.Client(new TBinaryProtocol(transport)); + transport.open(); + this.isConnected = true; + LOG.info("Connected to meta server: " + msUri); + } catch (URISyntaxException e) { + throw new HoodieException("Invalid meta server uri: " + uri, e); + } catch (TTransportException e) { + exception = e; + LOG.warn("Fail to connect to the meta server.", e); + } + } + if (!isConnected) { + throw new HoodieException("Fail to connect to the meta server.", exception); + } + } + + private boolean isLocalEmbeddedMetaServer(String uri) { + return uri == null ? true : uri.trim().isEmpty(); + } + + @Override + public Table getTable(String db, String tb) { + return exceptionWrapper(() -> this.client.get_table(db, tb)).get(); + } + + @Override + public void createTable(Table table) { + try { + this.client.create_table(table); + } catch (TException e) { + throw new HoodieException(e); + } + } + + public List<HoodieInstant> listInstants(String db, String tb, int commitNum) { + return exceptionWrapper(() -> this.client.list_instants(db, tb, commitNum).stream() + .map(EntryConvertor::fromTHoodieInstant) + .collect(Collectors.toList())).get(); + } + + public Option<byte[]> getInstantMeta(String db, String tb, HoodieInstant instant) { + ByteBuffer bytes = exceptionWrapper(() -> this.client.get_instant_meta(db, tb, EntryConvertor.toTHoodieInstant(instant))).get(); + Option<byte[]> res = bytes.capacity() == 0 ? Option.empty() : Option.of(bytes.array()); + return res; + } + + public String createNewTimestamp(String db, String tb) { + return exceptionWrapper(() -> this.client.create_new_instant_time(db, tb)).get(); + } + + public void createNewInstant(String db, String tb, HoodieInstant instant, Option<byte[]> content) { + exceptionWrapper(() -> this.client.create_new_instant_with_time(db, tb, EntryConvertor.toTHoodieInstant(instant), getByteBuffer(content))).get(); + } + + public void transitionInstantState(String db, String tb, HoodieInstant fromInstant, HoodieInstant toInstant, Option<byte[]> content) { + exceptionWrapper(() -> this.client.transition_instant_state(db, tb, + EntryConvertor.toTHoodieInstant(fromInstant), + EntryConvertor.toTHoodieInstant(toInstant), + getByteBuffer(content))).get(); + } + + public void deleteInstant(String db, String tb, HoodieInstant instant) { + exceptionWrapper(() -> this.client.delete_instant(db, tb, EntryConvertor.toTHoodieInstant(instant))).get(); + } + + // TODO: support snapshot creation + public FileStatus[] listFilesInPartition(String db, String tb, String partition, String timestamp) { + return null; Review Comment: Better return empty array instead of null -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
