[
https://issues.apache.org/jira/browse/HDFS-13245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16468533#comment-16468533
]
Yiqun Lin edited comment on HDFS-13245 at 5/9/18 8:14 AM:
----------------------------------------------------------
Thanks [~yiran] for updating the patch. Some comments from me:
*StateStoreSQLImpl .java*
1.In method {{insertRecord}} and {{updateRecord}},
{code:java}
} catch (UnsupportedEncodingException e) {
+ LOG.error("Cannot insert record for \"{}\", sql: \"{}\".",
+ tableName, sql, e);
+ } finally {
{code}
'Cannot insert record for' can be updated to ''Unsupported encoding error for'
2.
{code:java}
LOG.error("Cannot auto create table \"{}\"" +
+ ". type is \"{}\", sql: \"{}\".", dbType, tableName, sql, e);
{code}
{{. type}} should be {{, type}}
*hdfs-rbf-default.xml.java*
1. In description of {{dfs.federation.router.store.sql.jdbc.class}}, we miss
the JDBC class for HSSQLDB.
2.'Database connection url. such as' updates to {{Database connection url,
such as}}
3.'if value less than 1, use 1.' updates to {{If value less than 1, use 1.}}
4. For the setting
{{dfs.federation.router.store.sql.auto.create.schema.enabled}}, not so sure if
I understanding correctly. Would you add more description for this? But from my
perspective, this setting seems no needed.
*TestStateStore.java*
Also not sure this new test is really needed. I mean
{{TestStateStoreDriverBase}} may have cover these cases, for example driver
connection, etc.
*MockStateStoreSQLImpl.java*
Why we mock a new StateStoreSQLImpl that only testing for MSSQLServer? I
hardly see any difference between MockStateStoreSQLImpl and StateStoreSQLImpl.
was (Author: linyiqun):
Thanks [~yiran] for updating the patch. Some comments from me:
*StateStoreSQLImpl .java*
1.In method {{insertRecord}} and {{updateRecord}},
{code}
} catch (UnsupportedEncodingException e) {
+ LOG.error("Cannot insert record for \"{}\", sql: \"{}\".",
+ tableName, sql, e);
+ } finally {
{code}
'Cannot insert record for' can be updated to ''Unsupported encoding error for'
2.
{code}
LOG.error("Cannot auto create table \"{}\"" +
+ ". type is \"{}\", sql: \"{}\".", dbType, tableName, sql, e);
{code}
{{. type}} should be {{, type}}
*hdfs-rbf-default.xml.java*
1. In description of {{dfs.federation.router.store.sql.jdbc.class}}, we miss
the JDBC class for HSSQLDB.
2.'Database connection url. such as' updates to {{Database connection url, such
as}}
3.'if value less than 1, use 1.' updates to {{If value less than 1, use 1.}}
4. For the setting
{{dfs.federation.router.store.sql.auto.create.schema.enabled}}, not so sure if
I understanding correctly. Would you add more description for this? But from my
perspective, this seems no needed.
*TestStateStore.java*
Also not sure this new test is really needed. I mean
{{TestStateStoreDriverBase}} may have cover these cases, for example driver
connection, etc.
*MockStateStoreSQLImpl.java*
Why we mock a new StateStoreSQLImpl that only testing for MSSQLServer? I hardly
see any difference between MockStateStoreSQLImpl and StateStoreSQLImpl.
> RBF: State store DBMS implementation
> ------------------------------------
>
> Key: HDFS-13245
> URL: https://issues.apache.org/jira/browse/HDFS-13245
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs
> Reporter: maobaolong
> Assignee: Yiran Wu
> Priority: Major
> Attachments: HDFS-13245.001.patch, HDFS-13245.002.patch,
> HDFS-13245.003.patch, HDFS-13245.004.patch, HDFS-13245.005.patch,
> HDFS-13245.006.patch, HDFS-13245.007.patch, HDFS-13245.008.patch,
> HDFS-13245.009.patch, HDFS-13245.010.patch
>
>
> Add a DBMS implementation for the State Store.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
