Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/19530 )
Change subject: IMPALA-11940: [DOCS] Document manifest caching settings for Iceberg ...................................................................... Patch Set 4: (10 comments) http://gerrit.cloudera.org:8080/#/c/19530/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19530/2//COMMIT_MSG@9 PS2, Line 9: IMPALA-11658 implements Iceberg manifest caching for Impala. This patch > Nit: "implements Iceberg manifest caching for Impala." Done http://gerrit.cloudera.org:8080/#/c/19530/2//COMMIT_MSG@10 PS2, Line 10: adds documentation for configuring the cache(s). > Nit: "adds documentation for configuring the cache(s). Done http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml File docs/topics/impala_iceberg.xml: http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@614 PS2, Line 614: > Nit: contents? Done http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@617 PS2, Line 617: feature can be enabled for Impala by setting properties in Hadoop's core-site.xml > Nit: "as in the following" Done http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@631 PS2, Line 631: <codeph>iceberg.io-impl</codeph>: custom FileIO implementation to use in a > Do we recommend that this not be changed? Maybe we should say that? Done http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@632 PS2, Line 632: catalog. Must be set to enable manifest caching. Impala defaults to > Nit: "defaults to" Done http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@639 PS2, Line 639: > In the two cases above we started a new line after the title (in <codeph>), Removed newline after </codeph> in the other two. http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@640 PS2, Line 640: <codeph>iceberg.io.manifest.cache.max-total-bytes</codeph>: maximum total > Why would I want to set this to anything other than a high value? Oh. I see Yes. Added "and lower than <codeph>iceberg.io.manifest.cache.max-total-bytes</codeph>.". http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@652 PS2, Line 652: of a manifest file to be considered for caching in bytes. Manifest files with > Maybe move this before iceberg.io.manifest.cache.expiration-interval-ms Yes, reordered the example and bullet points. http://gerrit.cloudera.org:8080/#/c/19530/2/docs/topics/impala_iceberg.xml@659 PS2, Line 659: > Shouldn't it be HadoopCatalogs and HiveCatalogs, i.e. plural? Done -- To view, visit http://gerrit.cloudera.org:8080/19530 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idd761a81f5c81a25a5ec0889402f85157c23e9fe Gerrit-Change-Number: 19530 Gerrit-PatchSet: 4 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Andrew Sherman <asher...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 24 Feb 2023 16:22:53 +0000 Gerrit-HasComments: Yes
