You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/04/25 19:36:50 UTC

[GitHub] [incubator-pinot] elonazoulay opened a new pull request #6841: Refactor gcs

elonazoulay opened a new pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841


   ## Description
   <!-- Add a description of your PR here.
   A good description should include pointers to an issue or design document, etc.
   -->
   ## Upgrade Notes
   Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
   * [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR fix a zero-downtime upgrade introduced earlier?
   * [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
   
   Does this PR otherwise need attention when creating release notes? Things to consider:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   * [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
   ## Release Notes
   <!-- If you have tagged this as either backward-incompat or release-notes,
   you MUST add text here that you would like to see appear in release notes of the
   next release. -->
   
   <!-- If you have a series of commits adding or enabling a feature, then
   add this section only in final commit that marks the feature completed.
   Refer to earlier release notes to see examples of text.
   -->
   ## Documentation
   <!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
   See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
   -->
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] xiangfu0 merged pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
xiangfu0 merged pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841


   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#discussion_r620713631



##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/PinotFS.java
##########
@@ -108,7 +108,16 @@ public boolean move(URI srcUri, URI dstUri, boolean overwrite)
       // ensures the parent path of dst exists.
       try {
         Path parentPath = Paths.get(dstUri.getPath()).getParent();
-        URI parentUri = new URI(dstUri.getScheme(), dstUri.getHost(), parentPath.toString(), null);
+        /**
+         * Use authority instead of host if the value contains "_": uri.getHost() will be null
+         * This is related to https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6587184
+         * @see java.net.URI.Parser#parseHostname
+         */
+        String host = dstUri.getHost();
+        if (host == null) {
+          host = dstUri.getAuthority();
+        }
+        URI parentUri = new URI(dstUri.getScheme(), host, parentPath.toString(), null, null);

Review comment:
       It seems you're using the constructor with 5 parameters while the current one only has 4 params. It'd be good to clarify that.

##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/test/java/org/apache/pinot/plugin/filesystem/TestGcsUri.java
##########
@@ -0,0 +1,67 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.plugin.filesystem;
+
+import java.net.URI;
+import org.testng.annotations.Test;
+
+import static org.apache.pinot.plugin.filesystem.GcsUri.SCHEME;
+import static org.apache.pinot.plugin.filesystem.GcsUri.createGcsUri;
+import static org.testng.Assert.assertEquals;
+
+public class TestGcsUri {
+    @Test
+    public void testDifferentScheme() {
+        URI uri = URI.create("file://bucket/file");
+        GcsUri gcsUri = new GcsUri(uri);
+        assertEquals(gcsUri.getUri().getScheme(), SCHEME);
+    }
+
+    @Test
+    public void testNonAbsolutePath() {
+        // Relative path must be normalized to absolute path for gcs uri
+        // This is because the URI must have an absolute path component,
+        // ex. new URI("gs", "bucket",
+        GcsUri gcsUri = createGcsUri("bucket", "dir/file");
+        assertEquals(gcsUri, createGcsUri("bucket", "/dir/file"));
+    }
+
+    @Test
+    public void testUnderScoreBucketName() {
+        // This is why getAuthority is used instead of getHostName()
+        // see https://cloud.google.com/storage/docs/naming-buckets
+        // gcs allows _'s which would cause URI.getHost() to be null:
+        GcsUri gcsUri = new GcsUri(URI.create("gs://bucket_name/dir"));
+        assertEquals(gcsUri.getBucketName(), "bucket_name");
+    }
+
+    @Test
+    public void testRelativize() {

Review comment:
       It'd be good to add a test for the case when relativize returns false.

##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -34,104 +35,211 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
 import org.apache.commons.io.FileUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.filesystem.PinotFS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
-import static joptsimple.internal.Strings.isNullOrEmpty;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
+import static org.apache.pinot.plugin.filesystem.GcsUri.createGcsUri;
 
-public class GcsPinotFS  extends PinotFS {
+public class GcsPinotFS extends PinotFS {
   public static final String PROJECT_ID = "projectId";
   public static final String GCP_KEY = "gcpKey";
 
   private static final Logger LOGGER = LoggerFactory.getLogger(GcsPinotFS.class);
-  private static final String SCHEME = "gs";
-  private static final String DELIMITER = "/";
   private static final int BUFFER_SIZE = 128 * 1024;
-  private Storage storage;
+  // See https://cloud.google.com/storage/docs/json_api/v1/how-tos/batch
+  private static final int BATCH_LIMIT = 100;
+  private Storage _storage;
 
   @Override
   public void init(PinotConfiguration config) {
-    Credentials credentials;
+    LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
+    _storage = initializeStorage(config);
+  }
 
+  private Storage initializeStorage(PinotConfiguration config) {
+    checkArgument(!isNullOrEmpty(config.getProperty(PROJECT_ID)));
+    checkArgument(!isNullOrEmpty(config.getProperty(GCP_KEY)));
+    String projectId = config.getProperty(PROJECT_ID);
+    String gcpKey = config.getProperty(GCP_KEY);
     try {
-      StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
-      if (!isNullOrEmpty(config.getProperty(PROJECT_ID)) && !isNullOrEmpty(config.getProperty(GCP_KEY))) {
-        LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
-        String projectId = config.getProperty(PROJECT_ID);
-        String gcpKey = config.getProperty(GCP_KEY);
-        storageBuilder.setProjectId(projectId);
-        credentials = GoogleCredentials.fromStream(Files.newInputStream(Paths.get(gcpKey)));
-      } else {
-        LOGGER.info("Configs using default credential");
-        credentials = GoogleCredentials.getApplicationDefault();
-      }
-      storage = storageBuilder.setCredentials(credentials).build().getService();
+      return StorageOptions.newBuilder()
+          .setProjectId(projectId)
+          .setCredentials(GoogleCredentials.fromStream(Files.newInputStream(Paths.get(gcpKey))))
+          .build()
+          .getService();
     } catch (IOException e) {
       throw new UncheckedIOException(e);
     }
   }
 
-  private Bucket getBucket(URI uri) {
-    return storage.get(uri.getHost());
-  }
-
-  private Blob getBlob(URI uri) throws IOException {
+  @Override
+  public final boolean mkdir(URI uri)
+      throws IOException {
+    LOGGER.info("mkdir {}", uri);
     try {
-      URI base = getBase(uri);
-      String path = sanitizePath(base.relativize(uri).getPath());
-      return getBucket(uri).get(path);
-    } catch (StorageException e) {
+      GcsUri gcsUri = new GcsUri(uri);
+      // Prefix always returns a path with trailing /
+      String directoryPath = gcsUri.getPrefix();
+      // Do not create a bucket, different permissions are required
+      // The bucket should already exist
+      if (directoryPath.equals(GcsUri.DELIMITER)) {
+        return true;
+      }
+      if (existsDirectory(gcsUri)) {
+        return true;
+      }
+      Blob blob = getBucket(gcsUri).create(directoryPath, new byte[0]);
+      return blob.exists();
+    } catch (Exception e) {
       throw new IOException(e);
     }
   }
-  private boolean isPathTerminatedByDelimiter(URI uri) {
-    return uri.getPath().endsWith(DELIMITER);
+
+  @Override
+  public boolean delete(URI segmentUri, boolean forceDelete)
+      throws IOException {
+    LOGGER.info("Deleting uri {} force {}", segmentUri, forceDelete);
+    return delete(new GcsUri(segmentUri), forceDelete);
+  }
+
+  @Override
+  public boolean doMove(URI srcUri, URI dstUri)
+      throws IOException {
+    GcsUri srcGcsUri = new GcsUri(srcUri);
+    GcsUri dstGcsUri = new GcsUri(dstUri);
+    if (copy(srcGcsUri, dstGcsUri)) {
+      // Only delete if all files were successfully moved
+      return delete(srcGcsUri, true);
+    }
+    return false;
+  }
+
+  @Override
+  public boolean copy(URI srcUri, URI dstUri)
+      throws IOException {
+    LOGGER.info("Copying uri {} to uri {}", srcUri, dstUri);
+    return copy(new GcsUri(srcUri), new GcsUri(dstUri));
+  }
+
+  @Override
+  public boolean exists(URI fileUri)
+      throws IOException {
+    if (fileUri == null) {
+      return false;
+    }
+    return exists(new GcsUri(fileUri));
   }
 
-  private String normalizeToDirectoryPrefix(URI uri) throws IOException {
-    requireNonNull(uri, "uri is null");
-    URI strippedUri = getBase(uri).relativize(uri);
-    if (isPathTerminatedByDelimiter(strippedUri)) {
-      return sanitizePath(strippedUri.getPath());
+  @Override
+  public long length(URI fileUri)
+      throws IOException {
+    try {
+      GcsUri gcsUri = new GcsUri(fileUri);
+      checkState(!isPathTerminatedByDelimiter(gcsUri), "URI is a directory");
+      Blob blob = getBlob(gcsUri);
+      checkState(existsBlob(blob), "File '%s' does not exist", fileUri);
+      return blob.getSize();
+    } catch (Exception t) {
+      throw new IOException(t);
     }
-    return sanitizePath(strippedUri.getPath() + DELIMITER);
   }
 
-  private URI normalizeToDirectoryUri(URI uri) throws IOException {
-    if (isPathTerminatedByDelimiter(uri)) {
-      return uri;
+  @Override
+  public String[] listFiles(URI fileUri, boolean recursive)
+      throws IOException {
+    return listFiles(new GcsUri(fileUri), recursive);
+  }
+
+  @Override
+  public void copyToLocalFile(URI srcUri, File dstFile)
+      throws Exception {
+    LOGGER.info("Copy {} to local {}", srcUri, dstFile.getAbsolutePath());
+    checkState(!dstFile.isDirectory(), "File '%s' must not be a directory", dstFile);
+    FileUtils.forceMkdir(dstFile.getParentFile());
+    Blob blob = getBlob(new GcsUri(srcUri));
+    checkState(existsBlob(blob), "File '%s' does not exists", srcUri);
+    blob.downloadTo(dstFile.toPath());
+  }
+
+  @Override
+  public void copyFromLocalFile(File srcFile, URI dstUri)
+      throws Exception {
+    LOGGER.info("Copying file {} to uri {}", srcFile.getAbsolutePath(), dstUri);
+    GcsUri dstGcsUri = new GcsUri(dstUri);
+    checkState(!isPathTerminatedByDelimiter(dstGcsUri), "Path '%s' must be a filename", dstGcsUri);
+    Blob blob = getBucket(dstGcsUri).create(dstGcsUri.getPath(), new byte[0]);
+    WriteChannel writeChannel = blob.writer();
+    writeChannel.setChunkSize(BUFFER_SIZE);
+    ByteBuffer buffer = ByteBuffer.allocate(BUFFER_SIZE);
+    SeekableByteChannel channel = Files.newByteChannel(srcFile.toPath());
+    for (int bytesRead = channel.read(buffer); bytesRead != -1; bytesRead = channel.read(buffer)) {
+      buffer.flip();
+      writeChannel.write(buffer);
+      buffer.clear();
     }
+    writeChannel.close();
+  }
+
+  @Override
+  public boolean isDirectory(URI uri)
+      throws IOException {
+    return existsDirectory(new GcsUri(uri));
+  }
+
+  @Override
+  public long lastModified(URI uri)
+      throws IOException {
+    return getBlob(new GcsUri(uri)).getUpdateTime();
+  }
+
+  @Override
+  public boolean touch(URI uri)
+      throws IOException {
     try {
-      return new URI(uri.getScheme(), uri.getHost(), sanitizePath(uri.getPath() + DELIMITER), null);
-    } catch (URISyntaxException e) {
+      GcsUri gcsUri = new GcsUri(uri);

Review comment:
       Adding one more log here since touch is a write operation.

##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsUri.java
##########
@@ -0,0 +1,186 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.plugin.filesystem;
+
+import com.google.common.base.Supplier;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static com.google.common.base.Suppliers.memoize;
+import static java.util.Objects.requireNonNull;
+
+
+public class GcsUri {
+  public static final String SCHEME = "gs";
+  public static final String DELIMITER = "/";
+
+  private final URI uri;
+  private final Supplier<String> path;

Review comment:
       It'd be good to use _path here in order to keep the same convention as other classes. Same for the next following two variables.




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-826386376


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6841](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bf20e8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/ac5aee70bf25aa4680a115434066287e3456ae71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac5aee7) will **increase** coverage by `0.18%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6841/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6841      +/-   ##
   ============================================
   + Coverage     43.52%   43.71%   +0.18%     
     Complexity        7        7              
   ============================================
     Files          1421     1421              
     Lines         69142    69145       +3     
     Branches       9987     9988       +1     
   ============================================
   + Hits          30093    30224     +131     
   + Misses        36504    36398     -106     
   + Partials       2545     2523      -22     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.71% <0.00%> (+0.18%)` | `7.00 <0.00> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/pinot/spi/filesystem/PinotFS.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZmlsZXN5c3RlbS9QaW5vdEZTLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ion/groupby/AggregationGroupByTrimmingService.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L0FnZ3JlZ2F0aW9uR3JvdXBCeVRyaW1taW5nU2VydmljZS5qYXZh) | `66.66% <0.00%> (-28.99%)` | `0.00% <0.00%> (ø%)` | |
   | [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `73.52% <0.00%> (-11.77%)` | `0.00% <0.00%> (ø%)` | |
   | [...lter/predicate/NotInPredicateEvaluatorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL05vdEluUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `48.95% <0.00%> (-10.42%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/local/segment/index/readers/FloatDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvRmxvYXREaWN0aW9uYXJ5LmphdmE=) | `70.00% <0.00%> (-10.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...operator/filter/RangeIndexBasedFilterOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvUmFuZ2VJbmRleEJhc2VkRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `40.42% <0.00%> (-4.26%)` | `0.00% <0.00%> (ø%)` | |
   | [...rg/apache/pinot/core/query/utils/idset/IdSets.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS91dGlscy9pZHNldC9JZFNldHMuamF2YQ==) | `25.00% <0.00%> (-2.78%)` | `0.00% <0.00%> (ø%)` | |
   | [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `51.54% <0.00%> (-1.55%)` | `0.00% <0.00%> (ø%)` | |
   | [...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `76.25% <0.00%> (-1.25%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ac5aee7...5bf20e8](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] xiangfu0 commented on a change in pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#discussion_r620703094



##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -34,104 +35,211 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
 import org.apache.commons.io.FileUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.filesystem.PinotFS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
-import static joptsimple.internal.Strings.isNullOrEmpty;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
+import static org.apache.pinot.plugin.filesystem.GcsUri.createGcsUri;
 
-public class GcsPinotFS  extends PinotFS {
+public class GcsPinotFS extends PinotFS {
   public static final String PROJECT_ID = "projectId";
   public static final String GCP_KEY = "gcpKey";
 
   private static final Logger LOGGER = LoggerFactory.getLogger(GcsPinotFS.class);
-  private static final String SCHEME = "gs";
-  private static final String DELIMITER = "/";
   private static final int BUFFER_SIZE = 128 * 1024;
-  private Storage storage;
+  // See https://cloud.google.com/storage/docs/json_api/v1/how-tos/batch
+  private static final int BATCH_LIMIT = 100;
+  private Storage _storage;
 
   @Override
   public void init(PinotConfiguration config) {
-    Credentials credentials;
+    LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
+    _storage = initializeStorage(config);
+  }
 
+  private Storage initializeStorage(PinotConfiguration config) {
+    checkArgument(!isNullOrEmpty(config.getProperty(PROJECT_ID)));

Review comment:
       I think we still need to handle the default credential support here:
   ```
         StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
         if (!isNullOrEmpty(config.getProperty(PROJECT_ID)) && !isNullOrEmpty(config.getProperty(GCP_KEY))) {
           LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
           String projectId = config.getProperty(PROJECT_ID);
           String gcpKey = config.getProperty(GCP_KEY);
           storageBuilder.setProjectId(projectId);
           credentials = GoogleCredentials.fromStream(Files.newInputStream(Paths.get(gcpKey)));
         } else {
           LOGGER.info("Configs using default credential");
           credentials = GoogleCredentials.getApplicationDefault();
         }
         storage = storageBuilder.setCredentials(credentials).build().getService();
   ```




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-826386376


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6841](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (040e59b) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/ac5aee70bf25aa4680a115434066287e3456ae71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac5aee7) will **increase** coverage by `22.13%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6841/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #6841       +/-   ##
   =============================================
   + Coverage     43.52%   65.65%   +22.13%     
   - Complexity        7       12        +5     
   =============================================
     Files          1421     1427        +6     
     Lines         69142    69864      +722     
     Branches       9987    10110      +123     
   =============================================
   + Hits          30093    45869    +15776     
   + Misses        36504    20719    -15785     
   - Partials       2545     3276      +731     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `?` | `?` | |
   | unittests | `65.65% <100.00%> (?)` | `12.00 <0.00> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/pinot/spi/filesystem/PinotFS.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZmlsZXN5c3RlbS9QaW5vdEZTLmphdmE=) | `76.19% <100.00%> (+76.19%)` | `0.00 <0.00> (ø)` | |
   | [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [1097 more](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ac5aee7...040e59b](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-826386376


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6841](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4c11238) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/ac5aee70bf25aa4680a115434066287e3456ae71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac5aee7) will **increase** coverage by `22.42%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head 4c11238 differs from pull request most recent head 5bf20e8. Consider uploading reports for the commit 5bf20e8 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6841/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #6841       +/-   ##
   =============================================
   + Coverage     43.52%   65.95%   +22.42%     
   - Complexity        7       12        +5     
   =============================================
     Files          1421     1421               
     Lines         69142    69145        +3     
     Branches       9987     9988        +1     
   =============================================
   + Hits          30093    45602    +15509     
   + Misses        36504    20301    -16203     
   - Partials       2545     3242      +697     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `?` | `?` | |
   | unittests | `65.95% <100.00%> (?)` | `12.00 <0.00> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/pinot/spi/filesystem/PinotFS.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZmlsZXN5c3RlbS9QaW5vdEZTLmphdmE=) | `76.19% <100.00%> (+76.19%)` | `0.00 <0.00> (ø)` | |
   | [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../core/startree/plan/StarTreeTransformPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlVHJhbnNmb3JtUGxhbk5vZGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...core/startree/plan/StarTreeProjectionPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlUHJvamVjdGlvblBsYW5Ob2RlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/minion/executor/MinionTaskZkMetadataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhlY3V0b3IvTWluaW9uVGFza1prTWV0YWRhdGFNYW5hZ2VyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [1066 more](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ac5aee7...5bf20e8](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] xiangfu0 commented on a change in pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on a change in pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#discussion_r620703094



##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -34,104 +35,211 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
 import org.apache.commons.io.FileUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.filesystem.PinotFS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
-import static joptsimple.internal.Strings.isNullOrEmpty;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
+import static org.apache.pinot.plugin.filesystem.GcsUri.createGcsUri;
 
-public class GcsPinotFS  extends PinotFS {
+public class GcsPinotFS extends PinotFS {
   public static final String PROJECT_ID = "projectId";
   public static final String GCP_KEY = "gcpKey";
 
   private static final Logger LOGGER = LoggerFactory.getLogger(GcsPinotFS.class);
-  private static final String SCHEME = "gs";
-  private static final String DELIMITER = "/";
   private static final int BUFFER_SIZE = 128 * 1024;
-  private Storage storage;
+  // See https://cloud.google.com/storage/docs/json_api/v1/how-tos/batch
+  private static final int BATCH_LIMIT = 100;
+  private Storage _storage;
 
   @Override
   public void init(PinotConfiguration config) {
-    Credentials credentials;
+    LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
+    _storage = initializeStorage(config);
+  }
 
+  private Storage initializeStorage(PinotConfiguration config) {
+    checkArgument(!isNullOrEmpty(config.getProperty(PROJECT_ID)));

Review comment:
       I think we still need to handle the default credential support here:
   ```
   StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
         if (!isNullOrEmpty(config.getProperty(PROJECT_ID)) && !isNullOrEmpty(config.getProperty(GCP_KEY))) {
           LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
           String projectId = config.getProperty(PROJECT_ID);
           String gcpKey = config.getProperty(GCP_KEY);
           storageBuilder.setProjectId(projectId);
           credentials = GoogleCredentials.fromStream(Files.newInputStream(Paths.get(gcpKey)));
         } else {
           LOGGER.info("Configs using default credential");
           credentials = GoogleCredentials.getApplicationDefault();
         }
         storage = storageBuilder.setCredentials(credentials).build().getService();
   ```

##########
File path: pinot-plugins/pinot-file-system/pinot-gcs/src/main/java/org/apache/pinot/plugin/filesystem/GcsPinotFS.java
##########
@@ -34,104 +35,211 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.ByteBuffer;
 import java.nio.channels.Channels;
 import java.nio.channels.SeekableByteChannel;
 import java.nio.file.Files;
-import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.util.ArrayList;
+import java.util.List;
 import org.apache.commons.io.FileUtils;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.filesystem.PinotFS;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static java.util.Objects.requireNonNull;
-import static joptsimple.internal.Strings.isNullOrEmpty;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.String.format;
+import static org.apache.pinot.plugin.filesystem.GcsUri.createGcsUri;
 
-public class GcsPinotFS  extends PinotFS {
+public class GcsPinotFS extends PinotFS {
   public static final String PROJECT_ID = "projectId";
   public static final String GCP_KEY = "gcpKey";
 
   private static final Logger LOGGER = LoggerFactory.getLogger(GcsPinotFS.class);
-  private static final String SCHEME = "gs";
-  private static final String DELIMITER = "/";
   private static final int BUFFER_SIZE = 128 * 1024;
-  private Storage storage;
+  // See https://cloud.google.com/storage/docs/json_api/v1/how-tos/batch
+  private static final int BATCH_LIMIT = 100;
+  private Storage _storage;
 
   @Override
   public void init(PinotConfiguration config) {
-    Credentials credentials;
+    LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
+    _storage = initializeStorage(config);
+  }
 
+  private Storage initializeStorage(PinotConfiguration config) {
+    checkArgument(!isNullOrEmpty(config.getProperty(PROJECT_ID)));

Review comment:
       I think we still need to handle the default credential support here:
   ```
           StorageOptions.Builder storageBuilder = StorageOptions.newBuilder();
         if (!isNullOrEmpty(config.getProperty(PROJECT_ID)) && !isNullOrEmpty(config.getProperty(GCP_KEY))) {
           LOGGER.info("Configs are: {}, {}", PROJECT_ID, config.getProperty(PROJECT_ID));
           String projectId = config.getProperty(PROJECT_ID);
           String gcpKey = config.getProperty(GCP_KEY);
           storageBuilder.setProjectId(projectId);
           credentials = GoogleCredentials.fromStream(Files.newInputStream(Paths.get(gcpKey)));
         } else {
           LOGGER.info("Configs using default credential");
           credentials = GoogleCredentials.getApplicationDefault();
         }
         storage = storageBuilder.setCredentials(credentials).build().getService();
   ```




-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter commented on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-826386376


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6841](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bf20e8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/ac5aee70bf25aa4680a115434066287e3456ae71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac5aee7) will **decrease** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6841/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6841      +/-   ##
   ============================================
   - Coverage     43.52%   43.51%   -0.01%     
     Complexity        7        7              
   ============================================
     Files          1421     1421              
     Lines         69142    69145       +3     
     Branches       9987     9988       +1     
   ============================================
   - Hits          30093    30091       -2     
   - Misses        36504    36516      +12     
   + Partials       2545     2538       -7     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.51% <0.00%> (-0.01%)` | `7.00 <0.00> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/pinot/spi/filesystem/PinotFS.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZmlsZXN5c3RlbS9QaW5vdEZTLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ion/groupby/AggregationGroupByTrimmingService.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L0FnZ3JlZ2F0aW9uR3JvdXBCeVRyaW1taW5nU2VydmljZS5qYXZh) | `66.66% <0.00%> (-28.99%)` | `0.00% <0.00%> (ø%)` | |
   | [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `73.52% <0.00%> (-11.77%)` | `0.00% <0.00%> (ø%)` | |
   | [.../filter/predicate/InPredicateEvaluatorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL0luUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `55.17% <0.00%> (-11.50%)` | `0.00% <0.00%> (ø%)` | |
   | [...lter/predicate/NotInPredicateEvaluatorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL05vdEluUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `48.95% <0.00%> (-10.42%)` | `0.00% <0.00%> (ø%)` | |
   | [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `42.78% <0.00%> (-10.31%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/local/segment/index/readers/FloatDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvRmxvYXREaWN0aW9uYXJ5LmphdmE=) | `70.00% <0.00%> (-10.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../local/segment/index/readers/DoubleDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvRG91YmxlRGljdGlvbmFyeS5qYXZh) | `60.00% <0.00%> (-10.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ac5aee7...5bf20e8](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-commenter commented on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-826386376


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6841](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5bf20e8) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/ac5aee70bf25aa4680a115434066287e3456ae71?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ac5aee7) will **decrease** coverage by `0.00%`.
   > The diff coverage is `0.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6841/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #6841      +/-   ##
   ============================================
   - Coverage     43.52%   43.51%   -0.01%     
     Complexity        7        7              
   ============================================
     Files          1421     1421              
     Lines         69142    69145       +3     
     Branches       9987     9988       +1     
   ============================================
   - Hits          30093    30091       -2     
   - Misses        36504    36516      +12     
   + Partials       2545     2538       -7     
   ```
   
   | Flag | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | integration | `43.51% <0.00%> (-0.01%)` | `7.00 <0.00> (ø)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [.../java/org/apache/pinot/spi/filesystem/PinotFS.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZmlsZXN5c3RlbS9QaW5vdEZTLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
   | [...ion/groupby/AggregationGroupByTrimmingService.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9ncm91cGJ5L0FnZ3JlZ2F0aW9uR3JvdXBCeVRyaW1taW5nU2VydmljZS5qYXZh) | `66.66% <0.00%> (-28.99%)` | `0.00% <0.00%> (ø%)` | |
   | [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
   | [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
   | [...pache/pinot/core/util/SortedRangeIntersection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL1NvcnRlZFJhbmdlSW50ZXJzZWN0aW9uLmphdmE=) | `73.52% <0.00%> (-11.77%)` | `0.00% <0.00%> (ø%)` | |
   | [.../filter/predicate/InPredicateEvaluatorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL0luUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `55.17% <0.00%> (-11.50%)` | `0.00% <0.00%> (ø%)` | |
   | [...lter/predicate/NotInPredicateEvaluatorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvcHJlZGljYXRlL05vdEluUHJlZGljYXRlRXZhbHVhdG9yRmFjdG9yeS5qYXZh) | `48.95% <0.00%> (-10.42%)` | `0.00% <0.00%> (ø%)` | |
   | [...nction/DistinctCountBitmapAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9EaXN0aW5jdENvdW50Qml0bWFwQWdncmVnYXRpb25GdW5jdGlvbi5qYXZh) | `42.78% <0.00%> (-10.31%)` | `0.00% <0.00%> (ø%)` | |
   | [...t/local/segment/index/readers/FloatDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvRmxvYXREaWN0aW9uYXJ5LmphdmE=) | `70.00% <0.00%> (-10.00%)` | `0.00% <0.00%> (ø%)` | |
   | [.../local/segment/index/readers/DoubleDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvRG91YmxlRGljdGlvbmFyeS5qYXZh) | `60.00% <0.00%> (-10.00%)` | `0.00% <0.00%> (ø%)` | |
   | ... and [41 more](https://codecov.io/gh/apache/incubator-pinot/pull/6841/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ac5aee7...5bf20e8](https://codecov.io/gh/apache/incubator-pinot/pull/6841?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] xiangfu0 commented on pull request #6841: Refactor gcs

Posted by GitBox <gi...@apache.org>.
xiangfu0 commented on pull request #6841:
URL: https://github.com/apache/incubator-pinot/pull/6841#issuecomment-828702710


   This is an awesome fix! Many thanks @elonazoulay !


-- 
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org