Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @oc007us, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly enhances ArcadeDB's remote access capabilities by integrating gRPC. The primary goal is to move beyond the performance and reliability limitations of traditional REST/HTTP, offering a native, efficient, and type-safe communication layer. This enables high-volume streaming and bulk data ingestion workloads, aligning ArcadeDB with modern distributed and reactive application architectures. The changes aim to improve overall developer experience through generated client stubs and better performance characteristics.
Highlights
- gRPC Core Integration: Introduces full gRPC support for ArcadeDB, providing a strongly-typed, high-performance API for remote database access.
- Advanced Query Streaming: Enables streaming query execution with multiple retrieval modes: CURSOR (existing behavior), MATERIALIZE_ALL (materializes all results before batching), and PAGED (reissues queries per batch using LIMIT/SKIP for efficient paging).
- Flexible Insert Operations: Adds various insert mechanisms including BulkInsert (single request for large batches), InsertStream (client-streaming with server-side batching), and InsertBidirectional (full duplex with per-batch acknowledgements and transaction control).
- Comprehensive Record & Transaction Handling: Supports create, update, and lookup operations for records with schema-aware property handling, alongside robust transaction support via TransactionContext flags for begin, commit, and rollback, and enhanced type conversion utilities.
- Dedicated Client and Server Components: Includes a full client-side implementation in
RemoteGrpcDatabaseand server-side components (ArcadeDbGrpcService,GrpcServerPlugin) with interceptors for logging, metrics, authentication, and compression.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive gRPC support for ArcadeDB, which is a significant and valuable addition. The implementation covers unary, streaming, and bulk operations, along with a new gRPC client. My review focuses on ensuring the new code is robust, maintainable, and consistent. I've identified several areas for improvement, including the removal of redundant code and a duplicate Protobuf definition, more consistent exception handling, and making data conversions more robust. Addressing these points will help solidify this new API as a first-class citizen in the ArcadeDB ecosystem.
| @@ -0,0 +1,281 @@ | |||
| syntax = "proto3"; | |||
There was a problem hiding this comment.
This Protobuf file appears to be a duplicate and outdated version of the one located at grpc/src/main/proto/arcadedb-server.proto. Maintaining multiple versions of the API definition can lead to inconsistencies and build issues. This file should be removed to ensure there is a single source of truth for the gRPC service contract.
| private com.google.protobuf.Value toProtoValue(Object v) { | ||
| com.google.protobuf.Value.Builder vb = com.google.protobuf.Value.newBuilder(); | ||
| if (v == null) return vb.setNullValue(com.google.protobuf.NullValue.NULL_VALUE).build(); | ||
| if (v instanceof String s) return vb.setStringValue(s).build(); | ||
| if (v instanceof Boolean b) return vb.setBoolValue(b).build(); | ||
| if (v instanceof Number n) return vb.setNumberValue(n.doubleValue()).build(); | ||
| if (v instanceof java.time.Instant t) | ||
| return vb.setStringValue(t.toString()).build(); // or a Struct for Timestamp if you prefer | ||
| if (v instanceof java.util.Map<?,?> m) { | ||
| var sb = com.google.protobuf.Struct.newBuilder(); | ||
| for (var e : m.entrySet()) | ||
| sb.putFields(String.valueOf(e.getKey()), toProtoValue(e.getValue())); | ||
| return vb.setStructValue(sb.build()).build(); | ||
| } | ||
| if (v instanceof java.util.List<?> list) { | ||
| var lb = com.google.protobuf.ListValue.newBuilder(); | ||
| for (Object e : list) lb.addValues(toProtoValue(e)); | ||
| return vb.setListValue(lb.build()).build(); | ||
| } | ||
| return vb.setStringValue(String.valueOf(v)).build(); | ||
| } |
There was a problem hiding this comment.
The toProtoValue method converts unknown object types to a string using toString(). This can lead to silent data loss or incorrect data representation for complex types that are not explicitly handled (e.g., byte arrays, custom date/time types, etc.). Consider making this conversion more robust by either throwing an IllegalArgumentException for unsupported types or by adding explicit handling for more common types like byte[].
| public static class RemoteGrpcDatabaseWithCompression extends RemoteGrpcDatabase { | ||
|
|
||
| public RemoteGrpcDatabaseWithCompression(String server, int port, String databaseName, String userName, String userPassword, | ||
| ContextConfiguration configuration) { | ||
| super(server, port, databaseName, userName, userPassword, configuration); | ||
| } | ||
|
|
||
| @Override | ||
| protected ManagedChannel createChannel(String server, int port) { | ||
|
|
||
| return ManagedChannelBuilder.forAddress(server, port).usePlaintext() // No TLS/SSL | ||
| .compressorRegistry(CompressorRegistry.getDefaultInstance()) | ||
| .decompressorRegistry(DecompressorRegistry.getDefaultInstance()) | ||
| .maxInboundMessageSize(100 * 1024 * 1024) // 100MB max message size | ||
| .keepAliveTime(30, TimeUnit.SECONDS) // Keep-alive configuration | ||
| .keepAliveTimeout(10, TimeUnit.SECONDS).keepAliveWithoutCalls(true).build(); | ||
| } | ||
| } |
There was a problem hiding this comment.
This inner class RemoteGrpcDatabaseWithCompression seems redundant, as there is a separate top-level class grpc-client/src/main/java/com/arcadedb/remote/grpc/RemoteGrpcDatabaseWithCompression.java that serves the same purpose. The implementation here is also different from the top-level class, which can cause confusion. This inner class should likely be removed to avoid duplication and maintain a single source of truth for this functionality.
grpcw/src/main/java/com/arcadedb/server/grpc/ArcadeDbGrpcService.java
Outdated
Show resolved
Hide resolved
grpcw/src/main/java/com/arcadedb/server/grpc/ArcadeDbGrpcService.java
Outdated
Show resolved
Hide resolved
grpc-client/src/main/java/com/arcadedb/remote/grpc/RemoteGrpcDatabase.java
Outdated
Show resolved
Hide resolved
| private TransactionIsolation mapIsolationLevel(Database.TRANSACTION_ISOLATION_LEVEL level) { | ||
| switch (level) { | ||
| // case READ_UNCOMMITTED: | ||
| // return TransactionIsolation.READ_UNCOMMITTED; | ||
| case READ_COMMITTED: | ||
| return TransactionIsolation.READ_COMMITTED; | ||
| case REPEATABLE_READ: | ||
| return TransactionIsolation.REPEATABLE_READ; | ||
| // case SERIALIZABLE: | ||
| // return TransactionIsolation.SERIALIZABLE; | ||
| default: | ||
| return TransactionIsolation.READ_COMMITTED; | ||
| } | ||
| } |
There was a problem hiding this comment.
The mapIsolationLevel method is missing cases for READ_UNCOMMITTED and SERIALIZABLE isolation levels, which are commented out. If these levels are intentionally unsupported over gRPC, it would be clearer to throw an UnsupportedOperationException or add a comment explaining why they are omitted. Otherwise, they should be implemented.
grpc-client/src/main/java/com/arcadedb/remote/grpc/RemoteGrpcDatabase.java
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,422 @@ | |||
| # ArcadeDB gRPC Server Configuration Examples | |||
There was a problem hiding this comment.
This file appears to be a collection of notes and examples for gRPC configuration and usage. For better readability and maintainability, consider the following improvements:
- Rename to
README.md: This will enable Markdown rendering, making the headers, code blocks, and lists much easier to read. - Structure and Consolidate: The content has several repeated sections (e.g., Docker commands, configuration properties). Consolidating these into clearly defined sections would make the document more useful as a reference.
4 participants
What does this PR do?
This PR introduces full gRPC support for ArcadeDB, providing a strongly-typed, high-performance API for remote access to the database.
Key highlights include:
Streaming query execution with multiple retrieval modes:
LIMIT/SKIPfor efficient paging.Insert operations:
Record operations:
Transaction support:
TransactionContextflags forbegin,commit, androllbackto coordinate transactional operations across requests.Client-side API:
RemoteGrpcDatabasewith a simple, extensible API.Motivation
Related issues
N/A
Additional Notes
I don't yet have a set of client-side tests. These will be provided shortly.
Checklist
mvn -DskipTests installmvn clean package -Pdockerin thepackagemodule