Spark Iceberg manifest reports wrong parquet file sizes.

[dmgcodevil]

We are using spark iceberg and some iceberg manifest files report the wrong data file (parquet) size, it's ~ 2x larger than the actual parquet file size. The issue was found while investigating Presto Iceberg iss6369

the problem might be in ParquetWriter#length(), method

return writer.getPos() + (writeStore.isColumnFlushNeeded() ? writeStore.getBufferedSize() : 0);

maybe that's why a parquet file size in manifest > actual file size on drive

[dmgcodevil]

Quick update: I've reproduced the issue. will update the ticket as soon as I have more details

[dmgcodevil]

Ok, based on my debugging research the problem in this line

i.e. writer.getPos() returns the corrent size of the actual parquet file saved on disk, but writeStore.isColumnFlushNeeded() returns true and adds extra bytes.

In my case, the actual file size is 4351656, rows count = 470099

at the moment when length() is called , ColumnWriteStoreV1.rowCountForNextSizeCheck == 470100 so writeStore.isColumnFlushNeeded() returns true:

public boolean isColumnFlushNeeded() {
//     rowCount  == 470099
//     rowCountForNextSizeCheck == 470100
return rowCount + 1 >= rowCountForNextSizeCheck;
  }

maybe it can be fixed by adding an extra check :

  @Override
  public void close() throws IOException {
    flushRowGroup(true);
    writeStore.close();
    writer.end(metadata);
    this.closed = true; // new flag indicates that writer was closed


  @Override
  public long length() {
    try {
      if (closed) {
        return writer.getPos();
      } else {
        return writer.getPos() + (writeStore.isColumnFlushNeeded() ? writeStore.getBufferedSize() : 0);
      }
    } catch (IOException e) {
      throw new RuntimeIOException(e, "Failed to get file length");
    }
  }  

  }

cc @rdblue

[dmgcodevil]

UPDATE: I did some testing: 739578885 total rows, 900 parquet files, 1953 manifest files. No errors found.
The fix might be not ideal but I can confirm the now manifest files contain the correct data file sizes

[rdblue]

Sounds like you have a fix. Could you open a PR with it? Thanks!

[dmgcodevil]

@rdblue PR The fix fork, but I believe that there is a better way to fix it

[dmgcodevil]

@rdblue do you know when a new release will be cut off?

[rdblue]

We should have a release sometime this month.

[aokolnychyi]

@rdblue @dmgcodevil does this problem happen all the time or only in given cases? Shall we create a repair action that would fix the metadata by checking the fs?

[dmgcodevil]

@aokolnychyi I'd suggest creating a repair action. This is what we are about to do with our data.

[rdblue]

Why repair this? Is it causing problems with split planning?

[dmgcodevil]

@rdblue if you are using prestosql >= 348 you will face this problem

cc @aokolnychyi

[rdblue]

Thanks for the context, @dmgcodevil. That's is definitely a problem. I think we will want to have a Trino fix for it, with the ability to fix metadata as a work-around until that is released. If you have a utility to share that fixes the metadata, I think that would be useful for other people. Thanks!