lidavidm commented on code in PR #14382: URL: https://github.com/apache/arrow/pull/14382#discussion_r993951264
########## docs/source/java/dataset.rst: ########## @@ -213,18 +235,51 @@ be thrown during scanning. Native Object Resource Management ================================= As another result of relying on JNI, all components related to -``FileSystemDataset`` should be closed manually to release the corresponding -native objects after using. For example: +``FileSystemDataset`` should be closed manually or use try-with-resources to +release the corresponding native objects after using. For example: .. code-block:: Java - DatasetFactory factory = new FileSystemDatasetFactory(allocator, - NativeMemoryPool.getDefault(), FileFormat.PARQUET, uri); - Dataset dataset = factory.finish(); - Scanner scanner = dataset.newScan(new ScanOptions(100)); + String uri = "file:/opt/example.parquet"; + ScanOptions options = new ScanOptions(/*batchSize*/ 32768); + try ( + BufferAllocator allocator = new RootAllocator(); + DatasetFactory factory = new FileSystemDatasetFactory( + allocator, NativeMemoryPool.getDefault(), + FileFormat.PARQUET, uri); + Dataset dataset = factory.finish(); + Scanner scanner = dataset.newScan(options) + ) { + + // do something + + } catch (Exception e) { + e.printStackTrace(); + } - // do something +If user forgets to close them then native object leakage might be caused. - AutoCloseables.close(factory, dataset, scanner); +Development Guidelines +====================== -If user forgets to close them then native object leakage might be caused. +* Related to the note about ScanOptions batchSize argument: Let's try to read a Parquet file with gzip compression and 3 row groups: + + .. code-block:: + + # Let configure ScanOptions as: + ScanOptions options = new ScanOptions(/*batchSize*/ 32768); + + $ parquet-tools meta data4_3rg_gzip.parquet + file schema: schema + age: OPTIONAL INT64 R:0 D:1 + name: OPTIONAL BINARY L:STRING R:0 D:1 + row group 1: RC:4 TS:182 OFFSET:4 + row group 2: RC:4 TS:190 OFFSET:420 + row group 3: RC:3 TS:179 OFFSET:838 + + In this case, we are configuring ScanOptions batchSize argument limit to + 32768, it's greater than current 4 rows read on the file, then 4 is + used as a limit on the program execution instead of 32768 limit requested. + + In case the file had more than 32768 rows, then it would get split into + blocks of 32768 rows or less. Review Comment: ```suggestion Here, we set the batchSize in ScanOptions to 32768. Because that's greater than the number of rows in the next batch, which is 4 rows because the first row group has only 4 rows, then the program gets only 4 rows. The scanner will not combine smaller batches to reach the limit, but it will split large batches to stay under the limit. So in the case the row group had more than 32768 rows, it would get split into blocks of 32768 rows or less. ``` ########## docs/source/java/dataset.rst: ########## @@ -213,18 +235,51 @@ be thrown during scanning. Native Object Resource Management ================================= As another result of relying on JNI, all components related to -``FileSystemDataset`` should be closed manually to release the corresponding -native objects after using. For example: +``FileSystemDataset`` should be closed manually or use try-with-resources to +release the corresponding native objects after using. For example: .. code-block:: Java - DatasetFactory factory = new FileSystemDatasetFactory(allocator, - NativeMemoryPool.getDefault(), FileFormat.PARQUET, uri); - Dataset dataset = factory.finish(); - Scanner scanner = dataset.newScan(new ScanOptions(100)); + String uri = "file:/opt/example.parquet"; + ScanOptions options = new ScanOptions(/*batchSize*/ 32768); + try ( + BufferAllocator allocator = new RootAllocator(); + DatasetFactory factory = new FileSystemDatasetFactory( + allocator, NativeMemoryPool.getDefault(), + FileFormat.PARQUET, uri); + Dataset dataset = factory.finish(); + Scanner scanner = dataset.newScan(options) + ) { + + // do something + + } catch (Exception e) { + e.printStackTrace(); + } - // do something +If user forgets to close them then native object leakage might be caused. - AutoCloseables.close(factory, dataset, scanner); +Development Guidelines +====================== -If user forgets to close them then native object leakage might be caused. +* Related to the note about ScanOptions batchSize argument: Let's try to read a Parquet file with gzip compression and 3 row groups: Review Comment: ```suggestion * The ``batchSize`` argument of ``ScanOptions`` is a limit on the size of an individual batch. For example, let's try to read a Parquet file with gzip compression and 3 row groups: ``` Can we remove the list from this? It makes the formatting weird and there's only one item here anyways. ########## docs/source/java/dataset.rst: ########## @@ -213,18 +235,51 @@ be thrown during scanning. Native Object Resource Management ================================= As another result of relying on JNI, all components related to -``FileSystemDataset`` should be closed manually to release the corresponding -native objects after using. For example: +``FileSystemDataset`` should be closed manually or use try-with-resources to +release the corresponding native objects after using. For example: .. code-block:: Java - DatasetFactory factory = new FileSystemDatasetFactory(allocator, - NativeMemoryPool.getDefault(), FileFormat.PARQUET, uri); - Dataset dataset = factory.finish(); - Scanner scanner = dataset.newScan(new ScanOptions(100)); + String uri = "file:/opt/example.parquet"; + ScanOptions options = new ScanOptions(/*batchSize*/ 32768); + try ( + BufferAllocator allocator = new RootAllocator(); + DatasetFactory factory = new FileSystemDatasetFactory( + allocator, NativeMemoryPool.getDefault(), + FileFormat.PARQUET, uri); + Dataset dataset = factory.finish(); + Scanner scanner = dataset.newScan(options) + ) { + + // do something + + } catch (Exception e) { + e.printStackTrace(); + } - // do something +If user forgets to close them then native object leakage might be caused. - AutoCloseables.close(factory, dataset, scanner); +Development Guidelines Review Comment: IMO, "Development Guidelines" isn't a great name. Maybe this section can be combined with the one above, and renamed "Usage Notes" or something similar? And then we can have subheadings for "Native Object Resource Management" and "Batch Sizes". -- 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: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org