[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #406: Throw an exception when using Iceberg with Spark embedded metastore
rdblue commented on a change in pull request #406: Throw an exception when
using Iceberg with Spark embedded metastore
URL: https://github.com/apache/incubator-iceberg/pull/406#discussion_r318661845
##
File path: hive/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
##
@@ -44,6 +48,13 @@ protected HiveMetaStoreClient newClient() {
return new HiveMetaStoreClient(hiveConf);
} catch (MetaException e) {
throw new RuntimeMetaException(e, "Failed to connect to Hive Metastore");
+} catch (Throwable t) {
+ if (t.getMessage().contains("Another instance of Derby may have already
booted")) {
+LOG.error("Failed to start an embedded metastore because embedded
Derby supports only one" +
Review comment:
I think the error thrown should include this message. Logging is a good
idea, but users commonly miss or ignore log messages, so any helpful
information should be included in the exception that is thrown.
Can you throw a new `RuntimeMetaException` with this error message instead
of falling through to the "Failed to connect" one?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #406: Throw an exception when using Iceberg with Spark embedded metastore
rdblue commented on a change in pull request #406: Throw an exception when
using Iceberg with Spark embedded metastore
URL: https://github.com/apache/incubator-iceberg/pull/406#discussion_r317698991
##
File path: hive/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##
@@ -183,6 +183,12 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
}
threw = false;
} catch (TException | UnknownHostException e) {
+ if (e.getMessage().contains("Table/View 'HIVE_LOCKS' does not exist")) {
+LOG.error("Failed to acquire locks from metastore because 'HIVE_LOCKS'
doesn't exist, " +
+"this probably happened when using embedded metastore or doesn't
create transactional" +
+" meta table. Please reconfigure and start the metastore.", e);
Review comment:
This should also be thrown instead of logged.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #406: Throw an exception when using Iceberg with Spark embedded metastore
rdblue commented on a change in pull request #406: Throw an exception when
using Iceberg with Spark embedded metastore
URL: https://github.com/apache/incubator-iceberg/pull/406#discussion_r317698659
##
File path: hive/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
##
@@ -44,6 +48,14 @@ protected HiveMetaStoreClient newClient() {
return new HiveMetaStoreClient(hiveConf);
} catch (MetaException e) {
throw new RuntimeMetaException(e, "Failed to connect to Hive Metastore");
+} catch (Throwable t) {
+ if (t.getMessage().contains("Another instance of Derby may have already
booted")) {
+LOG.error("Another instance of Derby may have already booted. This is
happened when using" +
+" embedded metastore, which only supports one client at time.
Please change to use " +
+"other metastores which could support multiple clients, like
metastore service.", t);
Review comment:
Can we change the error message to something a bit shorter? How about:
"Failed to start an embedded metastore because Derby supports only one client
at a time. To fix this, use a metastore that supports multiple clients."
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #406: Throw an exception when using Iceberg with Spark embedded metastore
rdblue commented on a change in pull request #406: Throw an exception when
using Iceberg with Spark embedded metastore
URL: https://github.com/apache/incubator-iceberg/pull/406#discussion_r317698081
##
File path: hive/src/main/java/org/apache/iceberg/hive/HiveTableOperations.java
##
@@ -183,6 +183,12 @@ public void commit(TableMetadata base, TableMetadata
metadata) {
}
threw = false;
} catch (TException | UnknownHostException e) {
+ if (e.getMessage().contains("Table/View 'HIVE_LOCKS' does not exist")) {
+LOG.error("Failed to acquire locks from metastore because 'HIVE_LOCKS'
doesn't exist, " +
+"this probably happened when using embedded metastore or doesn't
create transactional" +
+" meta table. Please reconfigure and start the metastore.", e);
Review comment:
This recommendation isn't very helpful because it isn't clear what "the
metastore" is. How about this instead: "To fix this, use an alternative
metastore".
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request #406: Throw an exception when using Iceberg with Spark embedded metastore
rdblue commented on a change in pull request #406: Throw an exception when
using Iceberg with Spark embedded metastore
URL: https://github.com/apache/incubator-iceberg/pull/406#discussion_r317697046
##
File path: hive/src/main/java/org/apache/iceberg/hive/HiveClientPool.java
##
@@ -44,6 +48,14 @@ protected HiveMetaStoreClient newClient() {
return new HiveMetaStoreClient(hiveConf);
} catch (MetaException e) {
throw new RuntimeMetaException(e, "Failed to connect to Hive Metastore");
+} catch (Throwable t) {
+ if (t.getMessage().contains("Another instance of Derby may have already
booted")) {
+LOG.error("Another instance of Derby may have already booted. This is
happened when using" +
+" embedded metastore, which only supports one client at time.
Please change to use " +
+"other metastores which could support multiple clients, like
metastore service.", t);
Review comment:
It is fine to log this message, but I think the exception that is thrown
should also contain it.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
