You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/09/29 14:47:04 UTC
[GitHub] [hadoop-ozone] adoroszlai opened a new pull request #1454: HDDS-4285. Read is slow due to the frequent usage of UGI.getCurrentUser Call()
adoroszlai opened a new pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454
## What changes were proposed in this pull request?
Reduce the number of `getCurrentUser()` and `getTokens()` calls performed during some `ContainerProtocolCalls` operations. This is achieved by getting the tokens once in `BlockInputStream` and `BlockOutputStream` initialization, and passing them to `getBlock`, `putBlock`, `readChunk`, `writeChunk` calls.
https://issues.apache.org/jira/browse/HDDS-4285
## How was this patch tested?
Verified that time required for the repro unit test is improved.
Without the patch:
```
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 112.829 s - in org.apache.hadoop.ozone.client.io.TestKeyOutputStreamUnit
```
With the patch:
```
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 30.293 s - in org.apache.hadoop.ozone.client.io.TestKeyOutputStreamUnit
```
Regular CI:
https://github.com/adoroszlai/hadoop-ozone/runs/1181129921
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-706409317
Thanks @xiaoyuyao for reviewing and committing it, @ChenSammi and @linyiqun for the review, and @elek for finding the issue.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#discussion_r496889816
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java
##########
@@ -84,7 +86,8 @@ private ContainerProtocolCalls() {
* @throws IOException if there is an I/O error while performing the call
*/
public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
- DatanodeBlockID datanodeBlockID) throws IOException {
+ DatanodeBlockID datanodeBlockID,
+ Collection<Token<? extends TokenIdentifier>> tokens) throws IOException {
Review comment:
Thanks @linyiqun for the suggestion. Would
```java
@param tokens list of tokens the current user has, possibly including a token for this block
```
be OK, or do you have a better description for this param?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#discussion_r497636613
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockInputStream.java
##########
@@ -193,10 +196,11 @@ public synchronized void initialize() throws IOException {
if (token != null) {
UserGroupInformation.getCurrentUser().addToken(token);
Review comment:
Should we just pass down the token from the caller directly and bypass the UGI addToken/getTokens completely?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#discussion_r497641447
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java
##########
@@ -84,7 +86,8 @@ private ContainerProtocolCalls() {
* @throws IOException if there is an I/O error while performing the call
*/
public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
- DatanodeBlockID datanodeBlockID) throws IOException {
+ DatanodeBlockID datanodeBlockID,
+ Collection<Token<? extends TokenIdentifier>> tokens) throws IOException {
Review comment:
We can directly encode the pass in token for the specific block id without additional token selection.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701144316
@adoroszlai thanks for identify this performance issue. I'm not quite familar with tokens, since it's a user token, suppose it has a much longer liveness than input and output stream object instance, right?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701266985
> thanks for identify this performance issue
Credit goes to @elek who reported the problem, see HDDS-4285.
> I'm not quite familar with tokens
Neither am I, but we have some [doc](https://hadoop.apache.org/ozone/docs/1.0.0/security/secureozone.html#tokens-).
> since it's a user token, suppose it has a much longer liveness than input and output stream object instance, right?
It is a block token, specific to the block being written/read. I think it's short-lived.
https://github.com/apache/hadoop-ozone/blob/d1ac423b3fe35c5d363b348108512440f08ee5f1/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/token/OzoneBlockTokenIdentifier.java#L41-L45
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun edited a comment on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
linyiqun edited a comment on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701381759
>since it's a user token, suppose it has a much longer liveness than input and output stream object instance, right?
A good point that @ChenSammi mentioned, there is a setting to decide the expired time of block token and it's a long time as 1 day. This should be an enough time for client to read/write block data.
```xml
<property>
<name>hdds.block.token.expiry.time</name>
<value>1d</value>
<tag>OZONE, HDDS, SECURITY, TOKEN</tag>
<description>
Default value for expiry time of block token. This
setting supports multiple time unit suffixes as described in
dfs.heartbeat.interval. If no suffix is specified, then milliseconds is
assumed.
</description>
</property>
```
> No need to renew or revoke a block access token. when a cached block access token expires, the client simply get a new one. Block access token should be cached only in memory and never write to disk.
From this comment of OzoneBlockTokenIdentifier, current change also looks safe.
In additional, I'm +1 for current change. Thanks @adoroszlai for addressing the comment.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#discussion_r496819670
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java
##########
@@ -84,7 +86,8 @@ private ContainerProtocolCalls() {
* @throws IOException if there is an I/O error while performing the call
*/
public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
- DatanodeBlockID datanodeBlockID) throws IOException {
+ DatanodeBlockID datanodeBlockID,
+ Collection<Token<? extends TokenIdentifier>> tokens) throws IOException {
Review comment:
Can we update the javadoc comment of this method as we new added param tokens here?
The same suggestion for methods we changed below:
* putBlockAsync
* readChunk
* writeChunkAsync
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-705945770
@adoroszlai ， thanks for the explanation. +1.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on a change in pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#discussion_r497199494
##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/storage/ContainerProtocolCalls.java
##########
@@ -84,7 +86,8 @@ private ContainerProtocolCalls() {
* @throws IOException if there is an I/O error while performing the call
*/
public static GetBlockResponseProto getBlock(XceiverClientSpi xceiverClient,
- DatanodeBlockID datanodeBlockID) throws IOException {
+ DatanodeBlockID datanodeBlockID,
+ Collection<Token<? extends TokenIdentifier>> tokens) throws IOException {
Review comment:
@adoroszlai , above description looks good to me. Thanks.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-705945770
@adoroszlai ， thanks for the explanation. +1.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
linyiqun commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701381759
>since it's a user token, suppose it has a much longer liveness than input and output stream object instance, right?
A good point that @ChenSammi mentioned, there is a setting to decide the expired time of block token and it's a long time as 1 day. This should be an enough time for client to read/write block data. Suppose there also should be some service to renew expired token.
```xml
<property>
<name>hdds.block.token.expiry.time</name>
<value>1d</value>
<tag>OZONE, HDDS, SECURITY, TOKEN</tag>
<description>
Default value for expiry time of block token. This
setting supports multiple time unit suffixes as described in
dfs.heartbeat.interval. If no suffix is specified, then milliseconds is
assumed.
</description>
</property>
```
In additional, I'm +1 for current change. Thanks @adoroszlai for addressing the comment.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701520846
> Should we just pass down the token from the caller directly and bypass the UGI addToken/getTokens completely?
> We can directly encode the pass in token for the specific block id without additional token selection.
Thanks @xiaoyuyao for the comments. I was also wondering about this, but I didn't have enough background info to decide. If you feel comfortable with bypassing the UGI for the block token, I'm fine with making this simplification.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-705673221
Thanks @xiaoyuyao for the suggestion to directly pass down the block token. I have updated the patch accordingly.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-705673221
Thanks @xiaoyuyao for the suggestion to directly pass down the block token. I have updated the patch accordingly.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] linyiqun edited a comment on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
linyiqun edited a comment on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-701381759
>since it's a user token, suppose it has a much longer liveness than input and output stream object instance, right?
A good point that @ChenSammi mentioned, there is a setting to decide the expired time of block token and it's a long time as 1 day. This should be an enough time for client to read/write block data. Suppose there also should be some service to renew expired token.
```xml
<property>
<name>hdds.block.token.expiry.time</name>
<value>1d</value>
<tag>OZONE, HDDS, SECURITY, TOKEN</tag>
<description>
Default value for expiry time of block token. This
setting supports multiple time unit suffixes as described in
dfs.heartbeat.interval. If no suffix is specified, then milliseconds is
assumed.
</description>
</property>
```
In additional, I'm +1 for current change. Thanks @adoroszlai for addressing the comment.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1454: HDDS-4285. Read is slow due to frequent calls to UGI.getCurrentUser() and getTokens()
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1454:
URL: https://github.com/apache/hadoop-ozone/pull/1454#issuecomment-706248107
Thanks @adoroszlai for the update. LGTM, +1.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org