[jira] [Comment Edited] (HDFS-9732) Remove DelegationTokenIdentifier.toString() —for better logging output
[
https://issues.apache.org/jira/browse/HDFS-9732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213210#comment-15213210
]
Yongjun Zhang edited comment on HDFS-9732 at 3/26/16 9:29 PM:
--
Thanks [~aw].
I like your idea of checking over all places about the use of {{toString()}} in
CLI outputs. However, I think we can do it in separate jira as a master jira.
And let each subtask fix some related commands, or even fix one particular
issue (like this one). It would be really ambitious to use one jira to cover
all IMHO. But good thing is, we are aware of that this has been an
long-standing issue based on the discussion here, and we need keep this issue
in mind when reviewing jiras that may incur output incompatibility.
About method naming. Wonder if we can do {{toStringFrozen()}} or
{{toStringStable()}} and javadoc it as "stable API for backward compatibility,
currently only for CLI"? I can see that if we put the {{CLI}} keyword in the
name, then calling these methods for anything else would look awkward, I guess
there is still chance of calling these methods for something else.
The point we are trying to make is, *the method need to stable*, and do not
change it otherwise compatibility will be broken; and it's *not just* because
they are for CLI. For example, some methods called by CLI don't need to be
stable at all.
Probably we use name {{toStringStable()}} instead?
Thanks.
was (Author: yzhangal):
Thanks [~aw].
I like your idea of checking over all places about the use of {{toString()}} in
CLI outputs. However, I think we can do it in separate jira as a master jira.
And let each subtask fix some related commands, or even fix one particular
issue (like this one). It would be really ambitious to use one jira to cover
all IMHO. But good thing is, we are aware of that this has been issue for long
based on the discussion here.
About method naming. Wonder if we can do {{toStringFrozen()}} or
{{toStringStable()}} and javadoc it as "stable API for backward compatibility,
currently only for CLI"? I can see that if we put the {{CLI}} keyword in the
name, then calling these methods for anything else would look awkward, I guess
there is still chance of calling these methods for something else.
The point we are trying to make is, *the method need to stable*, and do not
change it otherwise compatibility will be broken; and it's *not just* because
they are for CLI. For example, some methods called by CLI don't need to be
stable at all.
Probably we use name {{toStringStable()}} instead?
Thanks.
> Remove DelegationTokenIdentifier.toString() —for better logging output
> --
>
> Key: HDFS-9732
> URL: https://issues.apache.org/jira/browse/HDFS-9732
> Project: Hadoop HDFS
> Issue Type: Improvement
>Affects Versions: 2.7.2
>Reporter: Steve Loughran
>Assignee: Yongjun Zhang
> Attachments: HADOOP-12752-001.patch, HDFS-9732.001.patch,
> HDFS-9732.002.patch
>
> Original Estimate: 0.5h
> Remaining Estimate: 0.5h
>
> HDFS {{DelegationTokenIdentifier.toString()}} adds some diagnostics info,
> owner, sequence number. But its superclass,
> {{AbstractDelegationTokenIdentifier}} contains a lot more information,
> including token issue and expiry times.
> Because {{DelegationTokenIdentifier.toString()}} doesn't include this data,
> information that is potentially useful for kerberos diagnostics is lost.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Comment Edited] (HDFS-9732) Remove DelegationTokenIdentifier.toString() —for better logging output
[
https://issues.apache.org/jira/browse/HDFS-9732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15213127#comment-15213127
]
Allen Wittenauer edited comment on HDFS-9732 at 3/26/16 5:49 PM:
-
While I can appreciate what you folks are saying, experience has shown that
Hadoop devs are not disciplined enough to use this appropriately. Incredibly
bad code gets tossed over the fence all the time that are obviously bad\-\-my
current favorite for 2.8 is still the "log metrics to files outside of the
metrics subsystem via log4j rather than just fixing the file metrics plug-in".
Something subtle like this difference is going to blow up big time. I still
believe that it should be harder to do the wrong thing, but I'll acquiesce in
order to move this forward.
That said, I'd still like to see:
a) audit *every* direct and indirect usage of toString to make sure it isn't
getting used for CLI output
b) the javadoc for the toString method needs to explicitly say that it is not
to be used for CLI output because it evolves and point to the relevant section
in the compat guidelines
c) the toStringFrozen should be renamed toStringCLI or something similar to
actually state what it does not what it is so that in 3.x it can be changed.
was (Author: aw):
While I can appreciate what you folks are saying, experience has shown that
Hadoop devs are not disciplined enough to use this appropriately. Incredibly
bad code gets tossed over the fence all the time that are obviously bad\-\-my
current is still the "log metrics to files outside of the metrics subsystem via
log4j rather than just fixing the file metrics plug-in". Something subtle like
this difference is going to blow up big time. I still believe that it should be
harder to do the wrong thing, but I'll acquiesce in order to move this forward.
That said, I'd still like to see:
a) audit *every* direct and indirect usage of toString to make sure it isn't
getting used for CLI output
b) the javadoc for the toString method needs to explicitly say that it is not
to be used for CLI output because it evolves and point to the relevant section
in the compat guidelines
c) the toStringFrozen should be renamed toStringCLI or something similar to
actually state what it does not what it is so that in 3.x it can be changed.
> Remove DelegationTokenIdentifier.toString() —for better logging output
> --
>
> Key: HDFS-9732
> URL: https://issues.apache.org/jira/browse/HDFS-9732
> Project: Hadoop HDFS
> Issue Type: Improvement
>Affects Versions: 2.7.2
>Reporter: Steve Loughran
>Assignee: Yongjun Zhang
> Attachments: HADOOP-12752-001.patch, HDFS-9732.001.patch,
> HDFS-9732.002.patch
>
> Original Estimate: 0.5h
> Remaining Estimate: 0.5h
>
> HDFS {{DelegationTokenIdentifier.toString()}} adds some diagnostics info,
> owner, sequence number. But its superclass,
> {{AbstractDelegationTokenIdentifier}} contains a lot more information,
> including token issue and expiry times.
> Because {{DelegationTokenIdentifier.toString()}} doesn't include this data,
> information that is potentially useful for kerberos diagnostics is lost.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Comment Edited] (HDFS-9732) Remove DelegationTokenIdentifier.toString() —for better logging output
[
https://issues.apache.org/jira/browse/HDFS-9732?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15212126#comment-15212126
]
Yongjun Zhang edited comment on HDFS-9732 at 3/25/16 5:35 PM:
--
Hi [~ste...@apache.org], thanks for reporting the issue here, and thanks
[~cnauroth] and [~aw] for the discussion.
I recently had an issue and reported HDFS-10211, Chris pointed out it's a
duplicate of this one. The attached patch seems for other issue. Do you mind I
work on this one?
As Chris stated in the comment above, to be backward compatible, we can add
--verbose switch to fetchdt to print out more details, but let the detail
behavior to be the same as before.
For places like to one I reported in HDFS-10211, I'd like to change it to print
verbose info.
Thanks.
was (Author: yzhangal):
Hi [~ste...@apache.org], thanks for reporting the issue here, and thanks
[~cnauroth] and [~aw] for the discussion.
I recently had an issue and reported HDFS-10211, Chris pointed out it's a
duplicate of this one. The attached patch seems for other issue. Do you mind I
work on this one?
To be backward compatible, we can add --verbose switch to fetchdt to print out
more details, but let the detail behavior to be the same as before.
For places like to one I reported in HDFS-10211, I'd like to change it to print
verbose info.
Thanks.
> Remove DelegationTokenIdentifier.toString() —for better logging output
> --
>
> Key: HDFS-9732
> URL: https://issues.apache.org/jira/browse/HDFS-9732
> Project: Hadoop HDFS
> Issue Type: Improvement
>Affects Versions: 2.7.2
>Reporter: Steve Loughran
> Attachments: HADOOP-12752-001.patch
>
> Original Estimate: 0.5h
> Remaining Estimate: 0.5h
>
> HDFS {{DelegationTokenIdentifier.toString()}} adds some diagnostics info,
> owner, sequence number. But its superclass,
> {{AbstractDelegationTokenIdentifier}} contains a lot more information,
> including token issue and expiry times.
> Because {{DelegationTokenIdentifier.toString()}} doesn't include this data,
> information that is potentially useful for kerberos diagnostics is lost.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
