eric-maynard commented on code in PR #1378:
URL: https://github.com/apache/polaris/pull/1378#discussion_r2046511236
##########
service/common/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java:
##########
@@ -1196,6 +1199,89 @@ private class BasePolarisTableOperations extends
BaseMetastoreTableOperations {
this.tableFileIO = defaultFileIO;
}
+ /**
+ * We override `current` as we aren't able to override `currentMetadata`
directly.
+ * `recentlyCommittedMetadata` allows a call to `current()` that directly
follows a call to
+ * `commit()` to return the metadata written by commit, skipping `refresh`.
+ */
+ @Override
+ public TableMetadata current() {
+ synchronized (recentlyCommittedMetadataLock) {
+ if (recentlyCommittedMetadata != null) {
+ TableMetadata tmp = recentlyCommittedMetadata;
+ LOGGER.debug("Skipped a refresh by re-using recently committed
metadata");
+ return tmp;
+ } else {
+ return super.current();
+ }
+ }
+ }
+
+ private void setRecentlyCommittedMetadata(TableMetadata
recentlyCommittedMetadata) {
+ synchronized (recentlyCommittedMetadataLock) {
+ if (recentlyCommittedMetadata != null
+ && recentlyCommittedMetadata.metadataFileLocation() != null) {
+ this.recentlyCommittedMetadata = recentlyCommittedMetadata;
+ }
+ }
+ }
+
+ private void clearRecentlyCommittedMetadata() {
+ synchronized (recentlyCommittedMetadataLock) {
+ recentlyCommittedMetadata = null;
+ }
+ }
+
+ /** We override this to clear `recentlyCommittedMetadata`. */
+ @Override
+ protected void requestRefresh() {
+ clearRecentlyCommittedMetadata();
+ super.requestRefresh();
+ }
+
+ /**
+ * We override this to set `recentlyCommittedMetadata` after the commit is
done. We also adjust
+ * the logic for detecting stale metadata, to allow
`recentlyCommittedMetadata` to be used here
+ * if it's returned by `current()`.
+ */
+ @Override
+ public void commit(TableMetadata base, TableMetadata metadata) {
+ synchronized (recentlyCommittedMetadataLock) {
+ TableMetadata currentMetadata = current();
+
+ // if the metadata is already out of date, reject it
+ if (base == null) {
+ if (currentMetadata != null) {
+ // when current is non-null, the table exists. but when base is
null, the commit is
+ // trying
+ // to create the table
+ throw new AlreadyExistsException("Table already exists: %s",
tableName());
+ }
+ } else if (base.metadataFileLocation() != null
+ &&
!base.metadataFileLocation().equals(currentMetadata.metadataFileLocation())) {
+ throw new CommitFailedException("Cannot commit: stale table
metadata");
+ } else if (base != currentMetadata) {
+ // This branch is different from BaseMetastoreTableOperations
+ LOGGER.debug(
+ "Base object differs from current metadata; proceeding because
metadata locations match");
+ } else if
(base.metadataFileLocation().equals(metadata.metadataFileLocation())) {
+ // if the metadata is not changed, return early
+ LOGGER.info("Nothing to commit.");
+ return;
+ }
+
+ long start = System.currentTimeMillis();
+ this.doCommit(base, metadata);
Review Comment:
Right now, I see an issue where the `metadataFileLocation` is not populated
after this call
--
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]
