You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "lsyldliu (via GitHub)" <gi...@apache.org> on 2023/02/20 03:35:43 UTC

[GitHub] [flink] lsyldliu opened a new pull request, #21966: [FLINK-29729][Parquet] Support including the configured properties from flink-conf.yaml during create ParquetReader

lsyldliu opened a new pull request, #21966:
URL: https://github.com/apache/flink/pull/21966

   ## What is the purpose of the change
   
   *As the issue description, currently we can't get Hadoop-related authentication information from flink-conf.yaml when create `ParquetReader` using S3AFileSystem in TM, but in JM, `SourceCoordinator` works well when getFileStatus from s3a. After taking a deep analysis of the source code, I found that on the JM side, we use Flink's own FileSystem to get `FileStatus`, which can get the authentication information from flink-conf.yaml; but on the TM side, we use Hadoop's own FileSystem to create footer, which can't get the authentication information, which is an inconsistent behavior and causes the problem of the issue description. So we should use Flink's FileSystem on both JM side and TM side.  This PR is a bugfix.*
   
   ## Brief change log
   
     - *Extracting the public class `ParquetInputFile`*
     - *Using the Flink FileSystem to create ParquetFileReader in TM*
   
   
   ## Verifying this change
   
   This change is already covered by existing tests in the parquet module.
   
   ## Does this pull request potentially affect one of the following parts:
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: (no)
     - The serializers: (no)
     - The runtime per-record code paths (performance sensitive): (no)
     - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
     - The S3 file system connector: (no)
   
   ## Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (JavaDocs)
   


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] flinkbot commented on pull request #21966: [FLINK-29729][Parquet] Support including the configured properties from flink-conf.yaml during create ParquetReader

Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1436276237

   <!--
   Meta data
   {
     "version" : 1,
     "metaDataEntries" : [ {
       "hash" : "d4b4469382934358da015ec57791da352070d79c",
       "status" : "UNKNOWN",
       "url" : "TBD",
       "triggerID" : "d4b4469382934358da015ec57791da352070d79c",
       "triggerType" : "PUSH"
     } ]
   }-->
   ## CI report:
   
   * d4b4469382934358da015ec57791da352070d79c UNKNOWN
   
   <details>
   <summary>Bot commands</summary>
     The @flinkbot bot supports the following commands:
   
    - `@flinkbot run azure` re-run the last Azure build
   </details>


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] stayrascal commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "stayrascal (via GitHub)" <gi...@apache.org>.
stayrascal commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1442721215

   @lsyldliu thanks a lot for looking at this issue, may I check that if we use flink filesytem instead of hadoop filesytem to open parquet file, so that flink fs can retrieve the configured credential info(or fs properties) from fink-conf.yaml automatically?


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] wuchong merged pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

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


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] wuchong commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1442759082

   @stayrascal thank you for helping to verifying it. 


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] lsyldliu commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "lsyldliu (via GitHub)" <gi...@apache.org>.
lsyldliu commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1442697375

   @wuchong Thanks for reviewing. Regarding your question, we can't verify the configuration in ParquetReader now because the `ParquetFileReader` doesn't contain the credential info. The credential info is only used when creating hadoop FileSystem,   I have checked the related code to create FileSystem, and it loads hadoop conf from flink-conf.yaml first, then create the corresponding FileSystem, refer to https://github.com/apache/flink/blob/0141f13ca801d5db45435d101a9c3ef83889bbc0/flink-filesystems/flink-s3-fs-base/src/main/java/org/apache/flink/fs/s3/common/AbstractS3FileSystemFactory.java#L126
   


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] lsyldliu commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "lsyldliu (via GitHub)" <gi...@apache.org>.
lsyldliu commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1442750732

   @stayrascal Very thanks you can make double-check. I think it works well because it works on JM side.


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] wuchong commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1450201534

   I will merge this fix first, @stayrascal please open new issues if there are still problems. 


-- 
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: issues-unsubscribe@flink.apache.org

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

[GitHub] [flink] wuchong commented on pull request #21966: [FLINK-29729][parquet] Fix credential info configured in flink-conf.yaml is lost during creating ParquetReader

Posted by "wuchong (via GitHub)" <gi...@apache.org>.
wuchong commented on PR #21966:
URL: https://github.com/apache/flink/pull/21966#issuecomment-1442017027

   Thanks for the contribution @lsyldliu , the change looks good to me. However, is it possible to add a test to verify the configuration can be passed to ParquetReader 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: issues-unsubscribe@flink.apache.org

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