[
https://issues.apache.org/jira/browse/HADOOP-13597?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15738918#comment-15738918
]
John Zhuge commented on HADOOP-13597:
-------------------------------------
Thanks a lot, Xiao !
bq. HttpServer2: Don't understand the skipSecretProvider. Could you explain and
add some comments/javadocs? Is this about to the AuthenticationFilter and it's
related secret providers startup tricks?
Remove it. Don't recall why I added it.
bq. HttpServer2: Possible to have createHttpsChannelConnector call
createHttpChannelConnector first, to reduce some duplicate codes? The
httpConfig.setSecureScheme(HTTPS_SCHEME); line seems reasonable to be inside
the method too.
Fixed.
bq. I found the AccessLoggingConfiguration class naming confusing. Looking at
the class javadoc didn't help much either - I only figured out until looking at
the code usage. Can't find a good replacement in my vocabulary (appreciate if
anyone else has better naming), but we should state in javadoc: 1 this a
configuration object that logs each access. 2. it redacts sensitive
information. Actually, maybe this is better to be a composites a Configuration
rather than inherits? At least whoever uses it later don't have to figure out
which method is an Override etc. (BTW, currently missing @Override annotations
on all methods, and set seems to be missing a super.set.)
Fixed. Is {{ConfigurationWithLogging}} a better name? This class is extremely
useful during development. Now it has served its purpose, I am ok to remove it.
bq. KMSWebServer: Nit - I think hadoop code mostly have static import on 1 line
and ignore the 80-char rules.
Fixed.
bq. KMSWebServer: Totally theoretical, it may be good to also have the isAlive
method, and probably add a waitActive-ish method in the MiniKMS, so interested
tests can call that and reduce flaky tests due to start up race.
Good idea. Can we create a follow-up JIRA to add it later when we modify the
flaky tests? Otherwise, the new method is unused and untested.
bq. Searching for tomcat, see several nitty references still:
AuthenticationFiler's var name isInitializedByTomcat, CommandsManual.md and
SecureMode.md, KMS's doc index.md.vm, and some code comments etc.
TODO to update the docs. Investigating
{{AuthenticationFiler#isInitializedByTomcat}}.
bq. For the passwords, agree with Robert on supportability. However I also see
similar code in DFSUtil (loadSslConfToHttpServerBuilder and getPassword). Was
these copied over? We should at least move that to a common util, and avoid
this level of duplication. This will probably leave us not having to change
Configuration, but adding a wrapper util. Or per Wei-Chiu's suggestion, maybe
not needed any more. Appreciate more javadocs here too, as to why such method
is needed.
I will start a separate thread. Quite a few duplicate code in HDFS, YARN, and
AWS.
bq. Didn't see answer to Allen's ask about unit tests. (Take a look at
hadoop-common-project/hadoop-common/src/test/scripts if you're wondering how
that's done).
Fixed.
bq. Nit: kms-site.xml is following other -site.xmls, to have a comment line
"put site-specific...", which is good. Please follow them closer to have this
line before the <configuration> element.
Fixed.
> Switch KMS from Tomcat to Jetty
> -------------------------------
>
> Key: HADOOP-13597
> URL: https://issues.apache.org/jira/browse/HADOOP-13597
> Project: Hadoop Common
> Issue Type: New Feature
> Components: kms
> Affects Versions: 2.6.0
> Reporter: John Zhuge
> Assignee: John Zhuge
> Attachments: HADOOP-13597.001.patch, HADOOP-13597.002.patch,
> HADOOP-13597.003.patch
>
>
> The Tomcat 6 we are using will reach EOL at the end of 2017. While there are
> other good options, I would propose switching to {{Jetty 9}} for the
> following reasons:
> * Easier migration. Both Tomcat and Jetty are based on {{Servlet
> Containers}}, so we don't have change client code that much. It would require
> more work to switch to {{JAX-RS}}.
> * Well established.
> * Good performance and scalability.
> Other alternatives:
> * Jersey + Grizzly
> * Tomcat 8
> Your opinions will be greatly appreciated.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
