the design of removeObjects

[KevinSmile]

as described in PR #809

MinioClient.removeObjects() is kind of strange and of low efficiency.

MinioClient.removeObjects() returns an Iterable.
This method is doing a DELETE job, but the actual delete job won't execute if you don't iterate over the returned Iterable-value.
Call MinioClient.removeObjects() alone deletes nothing, which is strange.

[vadmeste]

@KevinSmile thanks for your contribution. It is always better to discuss a new change before implementing it to save your time.

I don't see how this could be low efficiency, iterating should be much faster than calling a web service and delete 1000 elements.

Maybe it is your code which is doing something which takes time between iterations.

[KevinSmile]

@vadmeste thanks for for your replay!
I think you didn't get my point.

(There are 2 kinds of iterator in this issue.
One is the return value of MinioClient.removeObjects();
the other is about how to generate a batch-toDelete-subList.
batch-toDelete-subList's size should be less than 1000, which is the limit set by s3.
I believe batch operation is faster than one-by-one delete since it saves http-calls.
In fact, the original code also used 1000 as batch-size(in a while loop

minio-java/api/src/main/java/io/minio/MinioClient.java

Line 3019 in cd1109b

while (objectNameIter.hasNext() && i < 1000) {

)
As mentioned in #809, I believe using Guava's Iterables.partition and java8's stream to generate a batch-toDelete-subList is better than the original while-loop.

see batch-size limit:
https://docs.aws.amazon.com/AmazonS3/latest/API/API_DeleteObjects.html
https://stackoverflow.com/questions/54255990/cheapest-way-to-delete-2-billion-objects-from-s3-ia)

However, I believe the strange usage of Iterable is the key problem here.

It's a common practice that using iterator to traverse elements of a collection without exposing its underlying representation. I believe iterator should be used as kind of GET method, you should not do something hack (e.g. DELETE) during the iterating, as the original did in the hasNext and next method:

minio-java/api/src/main/java/io/minio/MinioClient.java

Line 3041 in cd1109b

public boolean hasNext() {

minio-java/api/src/main/java/io/minio/MinioClient.java

Line 3067 in cd1109b

public Result<DeleteError> next() {

If you just call MinioClient.removeObjects() without traverse the returned iterator after your call, you will not trigger the hasNext and next method, so the populate won't be executed, thus noting would be DELETED, which is strange.

minio-java/api/src/main/java/io/minio/MinioClient.java

Line 3014 in cd1109b

private synchronized void populate() {

[balamurugana]

@KevinSmile

As mentioned in #809, I believe using Guava's Iterables.partition and java8's stream to generate a batch-toDelete-subList is better than the original while-loop.

You could send this betterment PR separately

However, I believe the strange usage of Iterable is the key problem here.

It's a common practice that using iterator to traverse elements of a collection without exposing its underlying representation. I believe iterator should be used as kind of GET method, you should not do something hack (e.g. DELETE) during the iterating, as the original did in the hasNext and next method:

This is a design choice we made. Delete objects are unrecoverable, hence its not recommended to delete them in a background. That's the reason of this lazy eval.

[harshavardhana]

removeObjects is lazy eval clarified in #811 - closing this issue.