gargvishesh commented on code in PR #16047:
URL: https://github.com/apache/druid/pull/16047#discussion_r1514507754
##########
extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java:
##########
@@ -159,11 +167,23 @@ public void delete(final String bucket, final String
path) throws IOException
* @param bucket GCS bucket
* @param paths Iterable for absolute paths of objects to be deleted inside
the bucket
*/
- public void batchDelete(final String bucket, final Iterable<String> paths)
throws IOException
+ public void batchDelete(final String bucket, final Iterable<String> paths)
{
- List<Boolean> statuses = storage.get().delete(Iterables.transform(paths,
input -> BlobId.of(bucket, input)));
+ final List<Boolean> statuses =
storage.get().delete(Iterables.transform(paths, input -> BlobId.of(bucket,
input)));
Review Comment:
Have now added retry logic at places where batchDelete is used.
##########
extensions-core/google-extensions/src/test/java/org/apache/druid/storage/google/GoogleDataSegmentKillerTest.java:
##########
@@ -54,8 +51,7 @@ public class GoogleDataSegmentKillerTest extends
EasyMockSupport
private static final long TIME_0 = 0L;
private static final long TIME_1 = 1L;
private static final int MAX_KEYS = 1;
- private static final Exception RECOVERABLE_EXCEPTION = new
HttpResponseException.Builder(429, "recoverable", new HttpHeaders()).build();
Review Comment:
Corrected.
##########
extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleStorage.java:
##########
@@ -159,11 +167,23 @@ public void delete(final String bucket, final String
path) throws IOException
* @param bucket GCS bucket
* @param paths Iterable for absolute paths of objects to be deleted inside
the bucket
*/
- public void batchDelete(final String bucket, final Iterable<String> paths)
throws IOException
+ public void batchDelete(final String bucket, final Iterable<String> paths)
{
- List<Boolean> statuses = storage.get().delete(Iterables.transform(paths,
input -> BlobId.of(bucket, input)));
+ final List<Boolean> statuses =
storage.get().delete(Iterables.transform(paths, input -> BlobId.of(bucket,
input)));
Review Comment:
Added
##########
extensions-core/google-extensions/src/main/java/org/apache/druid/storage/google/GoogleDataSegmentKiller.java:
##########
@@ -70,12 +70,12 @@ public void kill(DataSegment segment) throws
SegmentLoadingException
// anymore, but we still delete them if exists.
deleteIfPresent(bucket, descriptorPath);
}
- catch (IOException e) {
+ catch (StorageException e) {
Review Comment:
Catching and re-throwing StorageException now
--
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-unsubscr...@druid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org
