HIVE-29529: Iceberg: Load table from cache HiveIcebergOutputCommitter#6394
HIVE-29529: Iceberg: Load table from cache HiveIcebergOutputCommitter#6394ayushtkn wants to merge 2 commits intoapache:masterfrom
Conversation
| JobConf conf = jobContext.getJobConf(); | ||
|
|
||
| table = Optional.ofNullable(table).orElseGet(() -> Catalogs.loadTable(conf, catalogProperties)); | ||
| table = Optional.ofNullable(table).orElseGet(() -> IcebergTableUtil.getTable(conf, catalogProperties)); |
There was a problem hiding this comment.
i think we need a fresh copy, to check conflicts, not a cached one
There was a problem hiding this comment.
should it not break at time of commit? if our commit is conflicting? Means this is again reloading the table & reading the metadata which we already read once.
Between this and before we commit there could be another commit which could create conflict. I don't think we are under lock
In that case I believe some test would fail....
There was a problem hiding this comment.
let's see, i think it was intentional. there was no conflict found during commit. but if that's not needed, we could save some calls
There was a problem hiding this comment.
cc @difin didn't you have some issue with that in a past?
|
| JobConf conf = jobContext.getJobConf(); | ||
|
|
||
| table = Optional.ofNullable(table).orElseGet(() -> Catalogs.loadTable(conf, catalogProperties)); | ||
| table = Optional.ofNullable(table).orElseGet(() -> IcebergTableUtil.getTable(conf, catalogProperties)); |
There was a problem hiding this comment.
with that change, if it works
Table snapshotTable = Optional.ofNullable(IcebergAcidUtil.getTransaction(table))
.map(Transaction::table)
.orElse(table);
is not longer needed and could be simplified
3 participants
What changes were proposed in this pull request?
Load table from cache in HiveIcebergOutputCommitter, whenever it is available
Why are the changes needed?
To avoid reloading the table
Does this PR introduce any user-facing change?
No
How was this patch tested?
CI