grantatspothero commented on PR #16207: URL: https://github.com/apache/iceberg/pull/16207#issuecomment-4399601951
> I get this, what I don't get is how this PR helps with achieving this. If a REST server sends a full snapshot log we still put that full log into the TableMetadata, this PR doesn't help you with reducing the memory pressure, then, right? Yes agreed, this PR is only helpful if REFS mode is defined per spec as "truncate both snapshots and snapshotLog to only snapshot entries referenced by refs". You are suggesting that is not the current definition of REFS mode per spec. I'm suggesting the spec is ambiguous as currently written and to meet the goals of REFS mode (slim metadata loading on clients) it might be useful to be more explicit in the spec. [spec: for REFS mode](https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L983-L994) ``` description: The snapshots to return in the body of the metadata. Setting the value to `all` would return the full set of snapshots currently valid for the table. Setting the value to `refs` would load all snapshots referenced by branches or tags. Default if no param is provided is `all`. required: false schema: type: string enum: [ all, refs ] ``` where "The snapshots" could refer to just snapshots array or snapshots array and snapshotLog. Right now it is ambiguous and some implementations have interpreted that differently. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
