szehon-ho commented on code in PR #6570:
URL: https://github.com/apache/iceberg/pull/6570#discussion_r1171929442
##########
hive-metastore/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java:
##########
@@ -572,8 +595,39 @@ private static boolean hiveEngineEnabled(TableMetadata
metadata, Configuration c
ConfigProperties.ENGINE_HIVE_ENABLED,
TableProperties.ENGINE_HIVE_ENABLED_DEFAULT);
}
+ /**
+ * Returns if the hive locking should be enabled on the table, or not.
+ *
+ * <p>The decision is made like this:
+ *
+ * <ol>
+ * <li>Table property value {@link TableProperties#HIVE_LOCK_ENABLED}
+ * <li>If the table property is not set then check the hive-site.xml
property value {@link
+ * ConfigProperties#LOCK_HIVE_ENABLED}
+ * <li>If none of the above is enabled then use the default value {@link
+ * TableProperties#HIVE_LOCK_ENABLED_DEFAULT}
+ * </ol>
+ *
+ * @param metadata Table metadata to use
+ * @param conf The hive configuration to use
+ * @return if the hive engine related values should be enabled or not
+ */
+ private static boolean hiveLockEnabled(TableMetadata metadata, Configuration
conf) {
Review Comment:
OK I think I get the difference as well, if I can summarize, we have two
choices
1. catalog config = NO_LOCK => start writing to all tables with no_lock,
unless table is set explicitly to LOCK.
2. catalog config = NO_LOCK => start writing to tables with no_lock only if
table set explicitly to NO_LOCK.
In both cases, we can achieve case 2/3, the issue is whether the user has to
set the tables explicitly to exclude or include them in new system.
You are right that maybe Option 2 is too much of a burden for the user to
manually mark all tables as NO_LOCK. If all Iceberg clients are upgraded in
the system, Option 2 may be a good check as then we are sure all of them will
write NO_LOCK together, but we come back to the same problem if some clients
are not upgraded. (which is what case 2/3 is trying to solve).
Let me think about it, but I think you are right that we cannot do better in
the mixed-version scenarios.
--
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]
