imbajin commented on code in PR #2982:
URL: https://github.com/apache/hugegraph/pull/2982#discussion_r3099063014
##########
hugegraph-server/hugegraph-rocksdb/src/main/java/org/apache/hugegraph/backend/store/rocksdb/RocksDBTables.java:
##########
@@ -208,6 +210,15 @@ public static Edge in(String database) {
protected BackendColumnIterator queryById(RocksDBSessions.Session
session, Id id) {
return this.getById(session, id);
}
+
+ @Override
+ protected BackendColumnIterator queryByIds(RocksDBSessions.Session
session,
+ Collection<Id> ids) {
+ if (!session.hasChanges()) {
+ return this.getByIds(session, ids);
Review Comment:
⚠️ Since this adds a new multi-get path for vertex/edge id queries, it would
be great to add a small RocksDB regression test here. The core cases that seem
worth covering are:
1. batch query with multiple existing ids
2. batch query with existing + missing ids mixed together
3. duplicate ids in the input
4. fallback to the old path when `session.hasChanges()` is true
##########
hugegraph-server/hugegraph-rocksdb/src/main/java/org/apache/hugegraph/backend/store/rocksdb/RocksDBTables.java:
##########
@@ -182,7 +182,9 @@ protected BackendColumnIterator
queryById(RocksDBSessions.Session session, Id id
@Override
protected BackendColumnIterator queryByIds(RocksDBSessions.Session
session,
Collection<Id> ids) {
- // TODO: use getByIds() after batch version multi-get is ready
+ if (!session.hasChanges()) {
+ return this.getByIds(session, ids);
Review Comment:
⚠️ This RocksDB-specific optimization looks reasonable to me as a focused
improvement on its own. One thing that may be worth clarifying is how HStore
should be handled, since it is the main distributed backend we maintain and its
batch-query path is a separate concern from RocksDB. If you think it makes
sense, we could either cover HStore in this PR as well, or keep this PR scoped
to RocksDB and follow up with a separate PR for HStore so the behavior and
performance trade-offs can be reviewed independently.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
