[jira] [Comment Edited] (HDFS-12977) Add stateId to RPC headers.
[
https://issues.apache.org/jira/browse/HDFS-12977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16399808#comment-16399808
]
Konstantin Shvachko edited comment on HDFS-12977 at 3/15/18 2:36 AM:
-
Great progress, Plamen. Few editorial comments
# {{AlignmentContext}} JavaDoc should say something like "Interface to align
the state between client and server via RPC communication".
#* Check for long lines
#* Needs annotations: {{@InterfaceAudience, @InterfaceStability}}
#* Abstract methods could use Javadoc too.
# {{ProtobufRpcEngine.Server}} Javadoc should say:
{code}
* @param alignmentContext provides server state info on client responses
{code}
#* The same in {{RpcEngine.getServer()}} and {{WritableRpcEngine.Server}}
# For {{RpcResponseHeaderProto}} we should probably just use {{stateId}} with
the comment that it is the last written global state id. In the response header
it is the last written GSI, but in request header it will become last seen id.
# {{GlobalStateIdContext}} needs Javadoc and annotations.
# {{TestClientTransactionId}} do we need to restart cluster for each test?
Would be better if start in {{@BeforeClass}} and shut down in {{@AfterClass}}
#* Very much like to see comments for test cases! It would be better to make
them Javadoc, and check for long lines.
# {{Client.setClientStateHandler()}} please update javadoc
#* {{DFSClient.lastSeenId}} -> {{lastSeenStateId}}
#* {{setClientStateHandler()}} should be updated to {{setAlignmentContext()}}
# How about {{ClientGCIContext}} instead of {{DfsClientAlignmentContext}}
#* needs Javadoc and annotations
# Also I recommend running checkstyle locally, I couldn't find artifacts on
Jenkins, but there may be diversions in the patch.
I think this completes the server part. We should think about setting GCI in
RequestHeader on the client side. In the next jira I assume.
was (Author: shv):
Great progress, Plamen. Few editorial comments
# {{AlignmentContext}} JavaDoc should say something like "Interface to align
the state between client and server via RPC communication".
#* Check for long lines
#* Needs annotations: {{@InterfaceAudience, @InterfaceStability}}
#* Abstract methods could use Javadoc too.
# {{ProtobufRpcEngine.Server}} Javadoc should say:
{code}
* @param alignmentContext provides server state info on client responses
{code}
#* The same in {{RpcEngine.getServer()}} and {{WritableRpcEngine.Server}}
# For {{RpcResponseHeaderProto}} we should probably just use {{stateId}} with
the comment that it is the last written global state id. In the response header
it is the last written GSI, but in request header it will become last seen id.
# {{GlobalStateIdContext}} needs Javadoc and annotations.
# {{TestClientTransactionId}} do we need to restart cluster for each test?
Would be better if start in {{@BeforeClass}} and shut down in {{@AfterClass}}
#* Very much like to see comments for test cases! It would be better to make
them Javadoc, and check for long lines.
# {{Client.setClientStateHandler()}} please update javadoc
#* {{DFSClient.lastSeenId}} -> {{lastSeenStateId}}
#* {{setClientStateHandler()}} should be updated to {{setAlignmentContext()}}
# How about {{ClientGCIContext}} instead of {{DfsClientAlignmentContext}}
#* needs Javadoc and annotations
I think this completes the server part. We should think about setting GCI in
RequestHeader on the client side. In the next jira I assume.
> Add stateId to RPC headers.
> ---
>
> Key: HDFS-12977
> URL: https://issues.apache.org/jira/browse/HDFS-12977
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ipc, namenode
>Reporter: Konstantin Shvachko
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS_12977.trunk.001.patch, HDFS_12977.trunk.002.patch,
> HDFS_12977.trunk.003.patch, HDFS_12977.trunk.004.patch,
> HDFS_12977.trunk.005.patch
>
>
> stateId is a new field in the RPC headers of NameNode proto calls.
> stateId is the journal transaction Id, which represents LastSeenId for the
> clients and LastWrittenId for NameNodes. See more in [reads from Standby
> design
> doc|https://issues.apache.org/jira/secure/attachment/12902925/ConsistentReadsFromStandbyNode.pdf].
--
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
[jira] [Comment Edited] (HDFS-12977) Add stateId to RPC headers.
[
https://issues.apache.org/jira/browse/HDFS-12977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16376246#comment-16376246
]
Konstantin Shvachko edited comment on HDFS-12977 at 2/25/18 9:23 PM:
-
Liked the patch Plamen, this is on the very much right track.
# Why do you need both the Observer and the Producer. It should be the same
interface, but have different client and server implementations, like
{{ClientProtocol}}.
# It looks to me that you don't need to push {{txidProvider}} all the way to
the {{Server}}. You can do it in the {{RPCEngine}}. In the end we are modifying
the Call, and the Server does not need to know about txIds.
# RPC is used by many Hadoop components, while transaction Id is only relevant
to NN communication. I thought we could wrap it into a more general class, like
{{CallStateContext}} (should think of a better name). It may be configurable to
allow different implementations. Then for NN it's definition will be in hdfs
package rather than in common.
was (Author: shv):
Liked the patch Palmen, this is on the very much right track.
# Why do you need both the Observer and the Producer. It should be the same
interface, but have different client and server implementations, like
{{ClientProtocol}}.
# It looks to me that you don't need to push {{txidProvider}} all the way to
the {{Server}}. You can do it in the {{RPCEngine}}. In the end we are modifying
the Call, and the Server does not need to know about txIds.
# RPC is used by many Hadoop components, while transaction Id is only relevant
to NN communication. I thought we could wrap it into a more general class, like
{{CallStateContext}} (should think of a better name). It may be configurable to
allow different implementations. Then for NN it's definition will be in hdfs
package rather than in common.
> Add stateId to RPC headers.
> ---
>
> Key: HDFS-12977
> URL: https://issues.apache.org/jira/browse/HDFS-12977
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ipc, namenode
>Reporter: Konstantin Shvachko
>Assignee: Plamen Jeliazkov
>Priority: Major
> Attachments: HDFS_12977.trunk.001.patch, HDFS_12977.trunk.002.patch
>
>
> stateId is a new field in the RPC headers of NameNode proto calls.
> stateId is the journal transaction Id, which represents LastSeenId for the
> clients and LastWrittenId for NameNodes. See more in [reads from Standby
> design
> doc|https://issues.apache.org/jira/secure/attachment/12902925/ConsistentReadsFromStandbyNode.pdf].
--
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
[jira] [Comment Edited] (HDFS-12977) Add stateId to RPC headers.
[ https://issues.apache.org/jira/browse/HDFS-12977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16372125#comment-16372125 ] Plamen Jeliazkov edited comment on HDFS-12977 at 2/23/18 5:34 PM: -- Attached new patch. I created two new interfaces in order to keep common and hdfs as separate modules: TransactionIdProvider and TransactionIdObserver – names are subject to change per review of course, but their names should suggest their function. The gist is that the FSNamesystem implements the TransactionIdProvider and the DFSClient implements the TransactionIdObserver. With some additional changes to RpcEngine I was able to make it so that via Server constructor I can pass a TransactionIdProvider and via Client I can pass a TransactionIdObserver. This makes it so we do not need to add a ton of try/finally across NameNodeRpcServer. I am not really happy with my code changes in Client currently; if anyone can provide some guidance on how I can better pass an object into its constructor I would greatly appreciate it. Regarding (1) I was also able to pass in HdfsServerConstants.INVALID_TXID by delegating it to the TransactionIdProvider via a method "getInvalidTransactionId" that is used to initialize the RpcResponse transactionId until it gets a valid txid. was (Author: zero45): Attached new patch. I created two new interfaces in order to keep common and hdfs as separate modules: TransactionIdProvider and TransactionIdObserver – names are subject to change per review of course, but their names should suggest their function. The gist is that the FSNamesystem implements the TransactionIdProvider and the DFSClient implements the TransactionIdObserver. With some additional changes to RpcEngine I was able to make it so that via Server constructor I can pass a TransactionIdProvider and via Client I can pass a TransactionIdObserver. This makes it so we do not need to add a ton of try/finally across NameNodeRpcServer. I am not really happy with the code in Client currently; if anyone can provide some guidance on how I can better pass an object into its constructor I would greatly appreciate it. Regarding (1) I was also able to pass in HdfsServerConstants.INVALID_TXID by delegating it to the TransactionIdProvider via a method "getInvalidTransactionId" that is used to initialize the RpcResponse transactionId until it gets a valid txid. > Add stateId to RPC headers. > --- > > Key: HDFS-12977 > URL: https://issues.apache.org/jira/browse/HDFS-12977 > Project: Hadoop HDFS > Issue Type: Sub-task > Components: ipc, namenode >Reporter: Konstantin Shvachko >Assignee: Plamen Jeliazkov >Priority: Major > Attachments: HDFS_12977.trunk.001.patch, HDFS_12977.trunk.002.patch > > > stateId is a new field in the RPC headers of NameNode proto calls. > stateId is the journal transaction Id, which represents LastSeenId for the > clients and LastWrittenId for NameNodes. See more in [reads from Standby > design > doc|https://issues.apache.org/jira/secure/attachment/12902925/ConsistentReadsFromStandbyNode.pdf]. -- 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
