Skip to content

Commit

Permalink
refactor: Remove batch concept from opendal (#5393)
Browse files Browse the repository at this point in the history
  • Loading branch information
Xuanwo authored Dec 6, 2024
1 parent c77a07a commit 43481f4
Show file tree
Hide file tree
Showing 24 changed files with 71 additions and 173 deletions.
4 changes: 2 additions & 2 deletions .github/services/s3/r2/disabled_action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# under the License.

name: r2
description: 'Behavior test for Cloudflare R2. This service is sponsored by @Xuanwo.'
description: "Behavior test for Cloudflare R2. This service is sponsored by @Xuanwo."

runs:
using: "composite"
Expand All @@ -38,6 +38,6 @@ runs:
run: |
cat << EOF >> $GITHUB_ENV
OPENDAL_S3_REGION=auto
OPENDAL_S3_BATCH_MAX_OPERATIONS=700
OPENDAL_S3_DELETE_MAX_SIZE=700
OPENDAL_S3_DISABLE_STAT_WITH_OVERRIDE=true
EOF
14 changes: 0 additions & 14 deletions bindings/c/include/opendal.h
Original file line number Diff line number Diff line change
Expand Up @@ -571,20 +571,6 @@ typedef struct opendal_capability {
* If operator supports presign write.
*/
bool presign_write;
/**
* If operator supports batch.
*/
bool batch;
/**
* If operator supports batch delete.
*/
bool batch_delete;
/**
* The max operations that operator supports in batch.
*
* If it is not set, this will be zero
*/
uintptr_t batch_max_operations;
/**
* If operator supports shared.
*/
Expand Down
12 changes: 0 additions & 12 deletions bindings/c/src/operator_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,15 +126,6 @@ pub struct opendal_capability {
/// If operator supports presign write.
pub presign_write: bool,

/// If operator supports batch.
pub batch: bool,
/// If operator supports batch delete.
pub batch_delete: bool,
/// The max operations that operator supports in batch.
///
/// If it is not set, this will be zero
pub batch_max_operations: usize,

/// If operator supports shared.
pub shared: bool,

Expand Down Expand Up @@ -263,9 +254,6 @@ impl From<core::Capability> for opendal_capability {
presign_read: value.presign_read,
presign_stat: value.presign_stat,
presign_write: value.presign_write,
batch: value.batch,
batch_delete: value.batch_delete,
batch_max_operations: value.batch_max_operations.unwrap_or(0),
shared: value.shared,
blocking: value.blocking,
}
Expand Down
12 changes: 0 additions & 12 deletions bindings/go/operator_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,18 +228,6 @@ func (c *Capability) PresignWrite() bool {
return c.inner.presignWrite == 1
}

func (c *Capability) Batch() bool {
return c.inner.batch == 1
}

func (c *Capability) BatchDelete() bool {
return c.inner.batchDelete == 1
}

func (c *Capability) BatchMaxOperations() uint {
return c.inner.batchMaxOperations
}

func (c *Capability) Shared() bool {
return c.inner.shared == 1
}
Expand Down
6 changes: 0 additions & 6 deletions bindings/go/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,6 @@ var (
&ffi.TypeUint8, // presign_read
&ffi.TypeUint8, // presign_stat
&ffi.TypeUint8, // presign_write
&ffi.TypeUint8, // batch
&ffi.TypeUint8, // batch_delete
&ffi.TypePointer, // batch_max_operations
&ffi.TypeUint8, // shared
&ffi.TypeUint8, // blocking
nil,
Expand Down Expand Up @@ -202,9 +199,6 @@ type opendalCapability struct {
presignRead uint8
presignStat uint8
presignWrite uint8
batch uint8
batchDelete uint8
batchMaxOperations uint
shared uint8
blocking uint8
}
Expand Down
5 changes: 1 addition & 4 deletions bindings/java/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn make_operator_info<'a>(env: &mut JNIEnv<'a>, info: OperatorInfo) -> Result<JO
fn make_capability<'a>(env: &mut JNIEnv<'a>, cap: Capability) -> Result<JObject<'a>> {
let capability = env.new_object(
"org/apache/opendal/Capability",
"(ZZZZZZZZZZZZZZZJJZZZZZZZZZZZZZZJZZ)V",
"(ZZZZZZZZZZZZZZZJJZZZZZZZZZZZZZZ)V",
&[
JValue::Bool(cap.stat as jboolean),
JValue::Bool(cap.stat_with_if_match as jboolean),
Expand Down Expand Up @@ -125,9 +125,6 @@ fn make_capability<'a>(env: &mut JNIEnv<'a>, cap: Capability) -> Result<JObject<
JValue::Bool(cap.presign_read as jboolean),
JValue::Bool(cap.presign_stat as jboolean),
JValue::Bool(cap.presign_write as jboolean),
JValue::Bool(cap.batch as jboolean),
JValue::Bool(cap.batch_delete as jboolean),
JValue::Long(convert::usize_to_jlong(cap.batch_max_operations)),
JValue::Bool(cap.shared as jboolean),
JValue::Bool(cap.blocking as jboolean),
],
Expand Down
21 changes: 0 additions & 21 deletions bindings/java/src/main/java/org/apache/opendal/Capability.java
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,6 @@ public class Capability {
*/
public final boolean presignWrite;

/**
* If operator supports batch.
*/
public final boolean batch;

/**
* If operator supports batch delete.
*/
public final boolean batchDelete;

/**
* The max operations that operator supports in batch.
*/
public final long batchMaxOperations;

/**
* If operator supports shared.
*/
Expand Down Expand Up @@ -225,9 +210,6 @@ public Capability(
boolean presignRead,
boolean presignStat,
boolean presignWrite,
boolean batch,
boolean batchDelete,
long batchMaxOperations,
boolean blocking,
boolean shared) {
this.stat = stat;
Expand Down Expand Up @@ -259,9 +241,6 @@ public Capability(
this.presignRead = presignRead;
this.presignStat = presignStat;
this.presignWrite = presignWrite;
this.batch = batch;
this.batchDelete = batchDelete;
this.batchMaxOperations = batchMaxOperations;
this.blocking = blocking;
this.shared = shared;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ public void testBlockingOperatorInfo() {
assertThat(info.fullCapability.writeCanAppend).isTrue();
assertThat(info.fullCapability.writeMultiMaxSize).isEqualTo(-1);
assertThat(info.fullCapability.writeMultiMinSize).isEqualTo(-1);
assertThat(info.fullCapability.batchMaxOperations).isEqualTo(-1);

assertThat(info.nativeCapability).isNotNull();
}
Expand All @@ -72,7 +71,6 @@ public void testOperatorInfo() {
assertThat(info.fullCapability.writeCanAppend).isFalse();
assertThat(info.fullCapability.writeMultiMaxSize).isEqualTo(-1);
assertThat(info.fullCapability.writeMultiMinSize).isEqualTo(-1);
assertThat(info.fullCapability.batchMaxOperations).isEqualTo(-1);

assertThat(info.nativeCapability).isNotNull();
}
Expand Down
6 changes: 0 additions & 6 deletions bindings/nodejs/generated.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,6 @@ export class Capability {
get presignStat(): boolean
/** If operator supports presign write. */
get presignWrite(): boolean
/** If operator supports batch. */
get batch(): boolean
/** If operator supports batch delete. */
get batchDelete(): boolean
/** The max operations that operator supports in batch. */
get batchMaxOperations(): bigint | null
/** If operator supports shared. */
get shared(): boolean
/** If operator supports blocking. */
Expand Down
18 changes: 0 additions & 18 deletions bindings/nodejs/src/capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,24 +227,6 @@ impl Capability {
self.0.presign_write
}

/// If operator supports batch.
#[napi(getter)]
pub fn batch(&self) -> bool {
self.0.batch
}

/// If operator supports batch delete.
#[napi(getter)]
pub fn batch_delete(&self) -> bool {
self.0.batch_delete
}

/// The max operations that operator supports in batch.
#[napi(getter)]
pub fn batch_max_operations(&self) -> Option<usize> {
self.0.batch_max_operations
}

/// If operator supports shared.
#[napi(getter)]
pub fn shared(&self) -> bool {
Expand Down
4 changes: 0 additions & 4 deletions bindings/python/python/opendal/__init__.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,5 @@ class Capability:
presign_stat: bool
presign_write: bool

batch: bool
batch_delete: bool
batch_max_operations: Optional[int]

shared: bool
blocking: bool
10 changes: 0 additions & 10 deletions bindings/python/src/capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,6 @@ pub struct Capability {
/// If operator supports presign write.
pub presign_write: bool,

/// If operator supports batch.
pub batch: bool,
/// If operator supports batch delete.
pub batch_delete: bool,
/// The max operations that operator supports in batch.
pub batch_max_operations: Option<usize>,

/// If operator supports shared.
pub shared: bool,

Expand Down Expand Up @@ -147,9 +140,6 @@ impl Capability {
presign_read: capability.presign_read,
presign_stat: capability.presign_stat,
presign_write: capability.presign_write,
batch: capability.batch,
batch_delete: capability.batch_delete,
batch_max_operations: capability.batch_max_operations,
shared: capability.shared,
blocking: capability.blocking,
}
Expand Down
6 changes: 0 additions & 6 deletions bindings/ruby/src/capability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,6 @@ define_accessors!(Capability, {
presign_read: bool,
presign_stat: bool,
presign_write: bool,
batch: bool,
batch_delete: bool,
batch_max_operations: Option<usize>,
shared: bool,
blocking: bool,
});
Expand Down Expand Up @@ -145,9 +142,6 @@ pub fn include(gem_module: &RModule) -> Result<(), Error> {
presign_read,
presign_stat,
presign_write,
batch,
batch_delete,
batch_max_operations,
blocking
});

Expand Down
2 changes: 1 addition & 1 deletion core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod tests {
/// unexpected struct/enum size change.
#[test]
fn assert_size() {
assert_eq!(40, size_of::<Operator>());
assert_eq!(32, size_of::<Operator>());
assert_eq!(296, size_of::<Entry>());
assert_eq!(272, size_of::<Metadata>());
assert_eq!(1, size_of::<EntryMode>());
Expand Down
3 changes: 1 addition & 2 deletions core/src/services/gcs/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,15 +385,14 @@ impl Access for GcsBackend {
},

delete: true,
delete_max_size: Some(100),
copy: true,

list: true,
list_with_limit: true,
list_with_start_after: true,
list_with_recursive: true,

batch: true,
batch_max_operations: Some(100),
presign: true,
presign_stat: true,
presign_read: true,
Expand Down
27 changes: 18 additions & 9 deletions core/src/services/oss/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,19 @@ impl OssBuilder {
}

/// Set maximum batch operations of this backend.
pub fn batch_max_operations(mut self, batch_max_operations: usize) -> Self {
self.config.batch_max_operations = Some(batch_max_operations);
#[deprecated(
since = "0.52.0",
note = "Please use `delete_max_size` instead of `batch_max_operations`"
)]
pub fn batch_max_operations(mut self, delete_max_size: usize) -> Self {
self.config.delete_max_size = Some(delete_max_size);

self
}

/// Set maximum delete operations of this backend.
pub fn delete_max_size(mut self, delete_max_size: usize) -> Self {
self.config.delete_max_size = Some(delete_max_size);

self
}
Expand Down Expand Up @@ -384,9 +395,9 @@ impl Builder for OssBuilder {

let signer = AliyunOssSigner::new(bucket);

let batch_max_operations = self
let delete_max_size = self
.config
.batch_max_operations
.delete_max_size
.unwrap_or(DEFAULT_BATCH_MAX_OPERATIONS);

Ok(OssBackend {
Expand All @@ -402,7 +413,7 @@ impl Builder for OssBuilder {
client,
server_side_encryption,
server_side_encryption_key_id,
batch_max_operations,
delete_max_size,
}),
})
}
Expand Down Expand Up @@ -464,7 +475,8 @@ impl Access for OssBackend {
write_with_user_metadata: true,

delete: true,
delete_max_size: Some(DEFAULT_BATCH_MAX_OPERATIONS),
delete_max_size: Some(self.core.delete_max_size),

copy: true,

list: true,
Expand All @@ -477,9 +489,6 @@ impl Access for OssBackend {
presign_read: true,
presign_write: true,

batch: true,
batch_max_operations: Some(self.core.batch_max_operations),

shared: true,

..Default::default()
Expand Down
6 changes: 6 additions & 0 deletions core/src/services/oss/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,13 @@ pub struct OssConfig {
/// Access key secret for oss.
pub access_key_secret: Option<String>,
/// The size of max batch operations.
#[deprecated(
since = "0.52.0",
note = "Please use `delete_max_size` instead of `batch_max_operations`"
)]
pub batch_max_operations: Option<usize>,
/// The size of max delete operations.
pub delete_max_size: Option<usize>,
/// If `role_arn` is set, we will use already known config as source
/// credential to assume role with `role_arn`.
pub role_arn: Option<String>,
Expand Down
2 changes: 1 addition & 1 deletion core/src/services/oss/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ pub struct OssCore {
pub client: HttpClient,
pub loader: AliyunLoader,
pub signer: AliyunOssSigner,
pub batch_max_operations: usize,
pub delete_max_size: usize,
}

impl Debug for OssCore {
Expand Down
Loading

0 comments on commit 43481f4

Please sign in to comment.