You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "TD-Sky (via GitHub)" <gi...@apache.org> on 2025/05/12 05:12:40 UTC

[PR] feat(core): share one redb database between different Operators [opendal]

TD-Sky opened a new pull request, #6173:
URL: https://github.com/apache/opendal/pull/6173

   # Which issue does this PR close?
   
   Closes #6172 .
   
   # What changes are included in this PR?
   
   - [x] `RedbBuilder::database(self, db: &Arc<redb::Database>) -> Self`
   - [x] Creating the specified table at build stage if it doesn't exist (It's really important because trying to get something in table is a common need, but errors will occur when the table doesn't exist)
   
   # Are there any user-facing changes?
   
   Users got a new method: `opendal::services::Redb::database(self, db: &Arc<redb::Database>) -> Self`


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(core): support sharing one redb database between different Operators [opendal]

Posted by "TD-Sky (via GitHub)" <gi...@apache.org>.
TD-Sky commented on code in PR #6173:
URL: https://github.com/apache/opendal/pull/6173#discussion_r2084893209


##########
core/src/services/redb/backend.rs:
##########
@@ -62,6 +67,12 @@ impl RedbBuilder {
         self.config.root = Some(path.into());
         self
     }
+
+    /// Set the database for Redb
+    pub fn database(mut self, db: &Arc<redb::Database>) -> Self {

Review Comment:
   There are no other ways to get `datadir` from `Arc<redb::Database>`. So I think it better to force users set the actual `datadir` when using `database()`, otherwise RedbBackend cannot give a `name` to `kv::Info`.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(core): support sharing one redb database between different Operators [opendal]

Posted by "TD-Sky (via GitHub)" <gi...@apache.org>.
TD-Sky commented on code in PR #6173:
URL: https://github.com/apache/opendal/pull/6173#discussion_r2085877878


##########
core/src/services/redb/backend.rs:
##########
@@ -62,6 +67,12 @@ impl RedbBuilder {
         self.config.root = Some(path.into());
         self
     }
+
+    /// Set the database for Redb
+    pub fn database(mut self, db: &Arc<redb::Database>) -> Self {

Review Comment:
   Done.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(core): support sharing one redb database between different Operators [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #6173:
URL: https://github.com/apache/opendal/pull/6173


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(core): support sharing one redb database between different Operators [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6173:
URL: https://github.com/apache/opendal/pull/6173#discussion_r2085780621


##########
core/src/services/redb/backend.rs:
##########
@@ -62,6 +67,12 @@ impl RedbBuilder {
         self.config.root = Some(path.into());
         self
     }
+
+    /// Set the database for Redb
+    pub fn database(mut self, db: &Arc<redb::Database>) -> Self {

Review Comment:
   We can use `table` as name.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] feat(core): share one redb database between different Operators [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6173:
URL: https://github.com/apache/opendal/pull/6173#discussion_r2084247591


##########
core/src/services/redb/backend.rs:
##########
@@ -62,6 +67,12 @@ impl RedbBuilder {
         self.config.root = Some(path.into());
         self
     }
+
+    /// Set the database for Redb
+    pub fn database(mut self, db: &Arc<redb::Database>) -> Self {

Review Comment:
   - Please move `database` before `datadir`.
   - Add comments for `datadir` and `database` to clarify their behaviors. For example, `datadir` and `database` should not be set simultaneously. If both are set, `database` will take precedence. Additionally, include comments for `datadir` to explain that it does not support sharing databases between different operation instances. If sharing and reuse of different tables are needed, `database` should be used instead.
   - `Arc<redb::Database>` should be enough.



##########
core/src/services/redb/backend.rs:
##########
@@ -79,9 +90,25 @@ impl Builder for RedbBuilder {
                 .with_context("service", Scheme::Redb)
         })?;
 
-        let db = redb::Database::create(&datadir_path).map_err(parse_database_error)?;
+        let db = if let Some(db) = self.database {
+            db
+        } else {
+            redb::Database::create(&datadir_path)
+                .map_err(parse_database_error)?
+                .into()
+        };
 
-        let db = Arc::new(db);
+        {

Review Comment:
   `redb` only allows one writer at a time, so it's better for us to check if the table exists before calling `begin_write`.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org