You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "amirmor1 (via GitHub)" <gi...@apache.org> on 2026/07/11 19:31:17 UTC
[PR] Core: Report metadata file size in commit metrics [iceberg]
amirmor1 opened a new pull request, #17164:
URL: https://github.com/apache/iceberg/pull/17164
## Summary
- Add an optional `metadata-file-size-bytes` metric to `CommitMetricsResult` and JSON serde.
- Capture metadata file size at write time via new additive APIs in `TableMetadataParser` and expose it through `TableOperations#metadataFileSizeInBytes()`.
- Populate `CommitReport` with this metric in `SnapshotProducer` when the table operations implementation provides the value.
- Add coverage in core metrics parser/report parser tests and Hadoop commit reporting tests.
- Update metrics reporting docs for the new commit metric.
## Why
- Exposes useful commit metadata size telemetry to `MetricsReporter` consumers.
- Captures size during write to avoid an extra metadata-file read after commit.
## Compatibility
- Existing `TableMetadataParser.write/overwrite/internalWrite` behavior remains intact.
- New APIs are additive (`writeAndReturnLength`, `overwriteAndReturnLength`).
- Metric is optional and may be `null` for table operations that cannot provide write-time size.
## Test plan
- [x] `./gradlew :iceberg-core:test --tests org.apache.iceberg.metrics.TestCommitMetricsResultParser`
- [x] `./gradlew :iceberg-core:test --tests org.apache.iceberg.metrics.TestCommitReportParser`
- [x] `./gradlew :iceberg-core:test --tests org.apache.iceberg.hadoop.TestHadoopCommits`
- [x] `./gradlew :iceberg-core:spotlessCheck`
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "amirmor1 (via GitHub)" <gi...@apache.org>.
amirmor1 commented on PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#issuecomment-5580024543
@manuzhang
All four of your comments are addressed:
- writeMetadata now returns the length directly, with the close scoped in a nested try
so the post-close read is explicit (same reason PuffinWriter#finish reads storedLength
after close). Dropped the metadataLength helper.
- Test renamed to drop the `test` prefix.
- Docs reworded to avoid the Java "null" detail.
- The size is now captured per commit attempt in SnapshotProducer, following the existing
newSnapshotId pattern, so it's no longer read off shared ops state at report time.
Added eachCommitReportsTheMetadataFileSizeItWrote to cover the scoping.
Still happy to thread the value through the commit path explicitly instead if you'd prefer
that shape — just let me know. No rush, and thanks for the review.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3772060986
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -506,4 +510,31 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void testCommitReportContainsMetadataFileSizeInBytes() {
Review Comment:
avoid using `test` prefix in test names.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "amirmor1 (via GitHub)" <gi...@apache.org>.
amirmor1 commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3774057593
##########
core/src/main/java/org/apache/iceberg/TableMetadataParser.java:
##########
@@ -125,13 +126,37 @@ public static void write(TableMetadata metadata, OutputFile outputFile) {
public static void internalWrite(
TableMetadata metadata, OutputFile outputFile, boolean overwrite) {
+ writeMetadata(metadata, outputFile, overwrite);
+ }
+
+ public static long writeAndReturnLength(TableMetadata metadata, OutputFile outputFile) {
+ PositionOutputStream outputStream = writeMetadata(metadata, outputFile, false);
+ return metadataLength(outputStream, outputFile.location());
+ }
+
+ public static long overwriteAndReturnLength(TableMetadata metadata, OutputFile outputFile) {
+ PositionOutputStream outputStream = writeMetadata(metadata, outputFile, true);
+ return metadataLength(outputStream, outputFile.location());
+ }
+
+ private static long metadataLength(PositionOutputStream stream, String location) {
+ try {
+ return stream.storedLength();
Review Comment:
I've reworked `writeMetadata` to return the length
directly, with the close scoped in a nested try so the ordering is explicit and commented,
and dropped the `metadataLength` helper.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3772070295
##########
docs/docs/metrics-reporting.md:
##########
@@ -41,6 +41,7 @@ A [`CommitReport`](https://github.com/apache/iceberg/blob/main/core/src/main/jav
* number of added/removed data/delete files
* number of added/removed equality/positional delete files
* number of added/removed equality/positional deletes
+* metadata file size in bytes for the committed table metadata file (optional, may be null depending on catalog implementation)
Review Comment:
"null" is a Java detail; "may be absent" reads better in docs
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#issuecomment-5260483805
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "raunaqmorarka (via GitHub)" <gi...@apache.org>.
raunaqmorarka commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r4060835176
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -520,6 +527,7 @@ public void commit() {
// to ensure that if a concurrent operation assigns the UUID, this operation will
// not fail.
taskOps.commit(base, updated.withUUID());
+ metadataFileSizeInBytes.set(taskOps.metadataFileSizeInBytes());
Review Comment:
This reads a field shared by every thread committing through the same TableOperations. If another thread's write lands between this thread's write and its commit, the size reported here belongs to the other thread's metadata file.
Suggest having the ops track (location, size) for its last write and only accepting the value here when the location matches `taskOps.current().metadataFileLocation()`. Otherwise leave it null.
##########
core/src/main/java/org/apache/iceberg/hadoop/HadoopTableOperations.java:
##########
@@ -152,7 +153,9 @@ public void commit(TableMetadata base, TableMetadata metadata) {
TableMetadataParser.Codec codec = TableMetadataParser.Codec.fromName(codecName);
String fileExtension = TableMetadataParser.getFileExtension(codec);
Path tempMetadataFile = metadataPath(UUID.randomUUID() + fileExtension);
- TableMetadataParser.write(metadata, io().newOutputFile(tempMetadataFile.toString()));
+ this.metadataFileSizeInBytes =
Review Comment:
This is set before `renameToFinal`, so a failed rename leaves the size of a file that never became current. Set it after the rename succeeds.
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -520,6 +527,7 @@ public void commit() {
// to ensure that if a concurrent operation assigns the UUID, this operation will
// not fail.
taskOps.commit(base, updated.withUUID());
+ metadataFileSizeInBytes.set(taskOps.metadataFileSizeInBytes());
Review Comment:
Inside a transaction `taskOps` is `TransactionTableOperations`, which does not write a metadata file, so this is always null there. The file is written later in `BaseTransaction` with no report. Either plumb the size through the transaction commit or call this out in the javadoc and docs.
##########
core/src/main/java/org/apache/iceberg/TableOperations.java:
##########
@@ -127,4 +127,17 @@ default long newSnapshotId() {
default boolean requireStrictCleanup() {
return true;
}
+
+ /**
+ * Returns the size in bytes of the metadata file written for the most recent commit handled by
+ * this table operations instance.
+ *
+ * <p>This value is optional and may be unavailable for implementations that do not write metadata
+ * files directly or cannot determine the final stored length at write time.
+ *
+ * @return metadata file size in bytes, or null if unavailable
+ */
+ default Long metadataFileSizeInBytes() {
Review Comment:
"Most recent commit handled by this instance" is not what callers get under concurrent commits or transactions. Please tighten the contract to the metadata file at `current().metadataFileLocation()`, and return null when that was not written by this instance.
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -508,4 +512,67 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void commitReportContainsMetadataFileSizeInBytes() {
Review Comment:
Only `HadoopTableOperations` is covered. `BaseMetastoreTableOperations` is the path every real catalog uses. Please add an equivalent test through `InMemoryCatalog`.
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -508,4 +512,67 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void commitReportContainsMetadataFileSizeInBytes() {
+ BaseTable baseTable = (BaseTable) table;
+ HadoopTableOperations ops = (HadoopTableOperations) baseTable.operations();
+
+ AtomicReference<CommitReport> capturedReport = new AtomicReference<>();
+ MetricsReporter reporter =
+ report -> {
+ if (report instanceof CommitReport) {
+ capturedReport.set((CommitReport) report);
+ }
+ };
+
+ Table tableWithReporter = new BaseTable(ops, baseTable.name(), reporter);
+ tableWithReporter.newFastAppend().appendFile(FILE_A).commit();
+
+ CommitReport commitReport = capturedReport.get();
+ assertThat(commitReport).isNotNull();
+ assertThat(reportedMetadataFileSize(commitReport)).isEqualTo(metadataFileSize(ops));
+ }
+
+ @Test
+ public void eachCommitReportsTheMetadataFileSizeItWrote() {
Review Comment:
Please also cover a retried commit: first attempt fails with `CommitFailedException`, second succeeds, and the report carries the second file's size. The concurrent commit helpers in this class should make that straightforward.
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -508,4 +512,67 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void commitReportContainsMetadataFileSizeInBytes() {
+ BaseTable baseTable = (BaseTable) table;
+ HadoopTableOperations ops = (HadoopTableOperations) baseTable.operations();
+
+ AtomicReference<CommitReport> capturedReport = new AtomicReference<>();
+ MetricsReporter reporter =
+ report -> {
+ if (report instanceof CommitReport) {
+ capturedReport.set((CommitReport) report);
+ }
+ };
+
+ Table tableWithReporter = new BaseTable(ops, baseTable.name(), reporter);
+ tableWithReporter.newFastAppend().appendFile(FILE_A).commit();
+
+ CommitReport commitReport = capturedReport.get();
+ assertThat(commitReport).isNotNull();
+ assertThat(reportedMetadataFileSize(commitReport)).isEqualTo(metadataFileSize(ops));
+ }
+
+ @Test
+ public void eachCommitReportsTheMetadataFileSizeItWrote() {
Review Comment:
Missing a transaction case. That would have caught the null result from `TransactionTableOperations`.
##########
core/src/test/java/org/apache/iceberg/metrics/TestCommitReportParser.java:
##########
@@ -300,4 +300,28 @@ public void roundTripSerdeWithMetadata() {
assertThat(CommitReportParser.fromJson(json)).isEqualTo(commitReport);
assertThat(json).isEqualTo(expectedJson);
}
+
+ @Test
+ public void roundTripSerdeWithMetadataFileSizeInCommitMetrics() {
+ String tableName = "roundTripTableName";
+ CommitReport commitReport =
+ ImmutableCommitReport.builder()
+ .tableName(tableName)
+ .snapshotId(23L)
+ .operation("DELETE")
+ .sequenceNumber(4L)
+ .commitMetrics(
+ ImmutableCommitMetricsResult.builder()
+ .metadataFileSizeInBytes(CounterResult.of(MetricsContext.Unit.BYTES, 123L))
+ .build())
+ .build();
+
+ String json = CommitReportParser.toJson(commitReport, true);
+
+ assertThat(json)
Review Comment:
Same here, compare the full expected JSON like the sibling tests.
##########
core/src/test/java/org/apache/iceberg/metrics/TestCommitMetricsResultParser.java:
##########
@@ -271,4 +272,20 @@ public void roundTripSerdeNoopCommitMetrics() {
assertThat(CommitMetricsResultParser.fromJson(json))
.isEqualTo(ImmutableCommitMetricsResult.builder().build());
}
+
+ @Test
+ public void roundTripSerdeWithMetadataFileSizeInBytes() {
+ CommitMetricsResult result =
+ ImmutableCommitMetricsResult.builder()
+ .metadataFileSizeInBytes(CounterResult.of(MetricsContext.Unit.BYTES, 123L))
+ .build();
+
+ String json = CommitMetricsResultParser.toJson(result, true);
+
+ assertThat(json)
Review Comment:
The other tests in this file compare against the exact expected JSON. Please do the same here so field placement and formatting are pinned.
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -508,4 +512,67 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void commitReportContainsMetadataFileSizeInBytes() {
Review Comment:
Please add a case with `write.metadata.compression-codec=gzip`. That is the case where reading the length only after close matters, and nothing exercises it.
##########
core/src/test/java/org/apache/iceberg/TestCommitReporting.java:
##########
@@ -90,6 +90,7 @@ public void addAndDeleteDataFiles() {
assertThat(metrics.manifestsCreated().value()).isEqualTo(1L);
assertThat(metrics.manifestsKept().value()).isEqualTo(0L);
assertThat(metrics.manifestsReplaced().value()).isEqualTo(1L);
+ assertThat(metrics.metadataFileSizeInBytes()).isNull();
Review Comment:
These `isNull` asserts only confirm `TestTables` reports nothing. They do not verify the feature. Consider dropping them, or move the positive assertions here once a metastore-backed test exists.
##########
docs/docs/metrics-reporting.md:
##########
@@ -41,6 +41,7 @@ A [`CommitReport`](https://github.com/apache/iceberg/blob/main/core/src/main/jav
* number of added/removed data/delete files
* number of added/removed equality/positional delete files
* number of added/removed equality/positional deletes
+* size in bytes of the table metadata file written by the commit, when available (some catalogs do not write metadata files directly)
Review Comment:
Also mention that commits made through a `Transaction` do not report this, unless that gets fixed in this PR.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "amirmor1 (via GitHub)" <gi...@apache.org>.
amirmor1 commented on PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#issuecomment-5263241771
not stale
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #17164: Core: Report metadata file size in commit metrics
URL: https://github.com/apache/iceberg/pull/17164
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3772234297
##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -55,6 +55,7 @@ public abstract class BaseMetastoreTableOperations extends BaseMetastoreOperatio
private String currentMetadataLocation = null;
private boolean shouldRefresh = true;
private int version = -1;
+ private volatile Long metadataFileSizeInBytes;
Review Comment:
the size is mutable per-ops-instance state, read later in `notifyListeners()`. If the same ops instance sees interleaved commits, the reported size can belong to a different metadata file than the reported snapshot
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#issuecomment-6329615708
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#issuecomment-6511713419
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "amirmor1 (via GitHub)" <gi...@apache.org>.
amirmor1 commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3774062619
##########
core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java:
##########
@@ -55,6 +55,7 @@ public abstract class BaseMetastoreTableOperations extends BaseMetastoreOperatio
private String currentMetadataLocation = null;
private boolean shouldRefresh = true;
private int version = -1;
+ private volatile Long metadataFileSizeInBytes;
Review Comment:
Agreed, this was the weakest part of the change. Reading shared ops state at report time
meant the value wasn't bound to the snapshot being reported, and it also left a stale size
in place on any commit path that doesn't write a metadata file.
I've moved the capture into the producer, following the `newSnapshotId` pattern already in
`SnapshotProducer#commit`:
```java
AtomicReference<Long> metadataFileSizeInBytes = new AtomicReference<>();
...
taskOps.commit(base, updated.withUUID());
metadataFileSizeInBytes.set(taskOps.metadataFileSizeInBytes());
...
notifyListeners(metadataFileSizeInBytes.get());
```
The size is now read in the same thread immediately after the commit attempt that produced
it, and it's scoped to a single producer. A concurrent commit through a shared ops instance
can no longer be attributed to this snapshot at report time, and stale values from earlier
commits are gone. Added `eachCommitReportsTheMetadataFileSizeItWrote` to cover the
per-commit scoping.
This still routes the value through a field on the ops instance, which I kept because
`TableOperations#commit` returns void and the write happens inside subclass `doCommit`
implementations.
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "amirmor1 (via GitHub)" <gi...@apache.org>.
amirmor1 commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3774053609
##########
core/src/test/java/org/apache/iceberg/hadoop/TestHadoopCommits.java:
##########
@@ -506,4 +510,31 @@ public void close() throws Exception {}
@Override
public void initialize(Map<String, String> properties) {}
}
+
+ @Test
+ public void testCommitReportContainsMetadataFileSizeInBytes() {
Review Comment:
Renamed to `commitReportContainsMetadataFileSizeInBytes`. Thanks.
##########
docs/docs/metrics-reporting.md:
##########
@@ -41,6 +41,7 @@ A [`CommitReport`](https://github.com/apache/iceberg/blob/main/core/src/main/jav
* number of added/removed data/delete files
* number of added/removed equality/positional delete files
* number of added/removed equality/positional deletes
+* metadata file size in bytes for the committed table metadata file (optional, may be null depending on catalog implementation)
Review Comment:
Agreed — reworded to "size in bytes of the table metadata file written by the commit,
when available (some catalogs do not write metadata files directly)".
--
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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Core: Report metadata file size in commit metrics [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #17164:
URL: https://github.com/apache/iceberg/pull/17164#discussion_r3772034981
##########
core/src/main/java/org/apache/iceberg/TableMetadataParser.java:
##########
@@ -125,13 +126,37 @@ public static void write(TableMetadata metadata, OutputFile outputFile) {
public static void internalWrite(
TableMetadata metadata, OutputFile outputFile, boolean overwrite) {
+ writeMetadata(metadata, outputFile, overwrite);
+ }
+
+ public static long writeAndReturnLength(TableMetadata metadata, OutputFile outputFile) {
+ PositionOutputStream outputStream = writeMetadata(metadata, outputFile, false);
+ return metadataLength(outputStream, outputFile.location());
+ }
+
+ public static long overwriteAndReturnLength(TableMetadata metadata, OutputFile outputFile) {
+ PositionOutputStream outputStream = writeMetadata(metadata, outputFile, true);
+ return metadataLength(outputStream, outputFile.location());
+ }
+
+ private static long metadataLength(PositionOutputStream stream, String location) {
+ try {
+ return stream.storedLength();
Review Comment:
This might be called on a closed stream since `writeMetadata` closes the `PositionOutputStream` via
try-with-resources and then returns 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@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org