[ 
https://issues.apache.org/jira/browse/HDFS-13245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16468533#comment-16468533
 ] 

Yiqun Lin commented on HDFS-13245:
----------------------------------

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: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to