[jira] [Comment Edited] (HDFS-13610) [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove unnecessary dummy sync
[
https://issues.apache.org/jira/browse/HDFS-13610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16540964#comment-16540964
]
Konstantin Shvachko edited comment on HDFS-13610 at 7/12/18 1:15 AM:
-
For [~csun]'s comments:
(1) {{lowestTxnId}} is initialized to {{-1}}, so it seems we need to check if
it is negative.
(2) Layout versions are always negative numbers. So using {{0}} instead of
{{Integer.MAX_VALUE}} as initial value is appropriate and preferable.
(3) Actually the entire log message in {{updateLayoutVersion()}} is confusing.
It should state the oldLV, newLV, and newStartTxn. The LVs can jump, so it's
better to know both.
was (Author: shv):
For [~csun]'s comments:
(1) {{lowestTxnId}} is initialized to {{-1}}, so it seems we need to check if
it is negative.
(2) Layout versions are always a negative numbers. So using {{0}} instead of
{{Integer.MAX_VALUE}} as initial value is appropriate and preferrable.
(3) Actually the entire log message in {{updateLayoutVersion()}} is confusing.
It should state the oldLV, newLV, and newStartTxn. The LVs can jump, so it's
better to know both.
> [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove
> unnecessary dummy sync
> --
>
> Key: HDFS-13610
> URL: https://issues.apache.org/jira/browse/HDFS-13610
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, journal-node, namenode
>Reporter: Erik Krogen
>Assignee: Erik Krogen
>Priority: Major
> Attachments: HDFS-13610-HDFS-12943.000.patch,
> HDFS-13610-HDFS-12943.001.patch, HDFS-13610-HDFS-12943.002.patch,
> HDFS-13610-HDFS-12943.003.patch
>
>
> See HDFS-13150 for full design.
> This JIRA is targeted at cleanup tasks:
> * Add in integration testing. We can expand {{TestStandbyInProgressTail}}
> * Documentation in HDFSHighAvailabilityWithQJM
> * Remove the dummy sync added as part of HDFS-10519; it is unnecessary since
> now in-progress tailing does not rely on the JN committedTxnId
> A few bugs are also fixed:
> * Due to changes in HDFS-13609 to enable use of the RPC mechanism whenever
> inProgressOK is true, there were codepaths which would use the RPC mechanism
> even when dfs.ha.tail-edits.in-progress was false, meaning that the JNs did
> not enable the cache. Update the QJM logic to only use selectRpcInputStreams
> if this config is true.
> * Fix a false error logged when the layout version changes
> * Fix the logging when a layout version change occurs to avoid printing out a
> placeholder value (Integer.MAX_VALUE)
--
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-13610) [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove unnecessary dummy sync
[
https://issues.apache.org/jira/browse/HDFS-13610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16530191#comment-16530191
]
Chao Sun edited comment on HDFS-13610 at 7/2/18 5:21 PM:
-
Thanks [~xkrogen]! A few of comments (mostly nits):
1. In {{JournaledEditsCache}}, do we need the check: {{lowestTxnId < 0}}?
seems the cache is already initialized in constructor.
2. nit: Can we define a static variable such as {{INVALID_LAYOUT_VERSION}} to
store the {{Integer.MAX_VALUE}}?
3. nit: {{Journal.LOG.info(logMsg.toString());}}: the {{toString}} is not
necessary?
4. Why we need {{getJournaledEditsCache()}} in {{JournalTestUtil}}? can we
replace it with {{journal.getJournaledEditsCache();}}?
5. nit: Can we also update {{JournaledEditsCache}} to add an empty line
between the Java doc and method parameters?
6. nit: in {{JournalTestUtil}}, maybe remove the extra empty line after
{{package org.apache.hadoop.hdfs.qjournal.server;}}?
7. nit: in Javadoc for {{JournalTestUtil#corruptJournaledEditsCache()}}, change
"simulated" to "simulate", and "transaction idea" to "transaction ID".
8. wonder if we can add information that the cache has been re-initialized to
the log message in a few places where the {{initialize}} is called, i.e., when
the cache is out-of-sync, and that the edit batch is too large to fit in.
was (Author: csun):
Thanks [~xkrogen]! A few of comments (mostly nits):
1. In {{JournaledEditsCache}}, do we need the check: {{lowestTxnId < 0}}?
seems the cache is already initialized in constructor.
2. nit: Can we define a static variable such as {{INVALID_LAYOUT_VERSION}} to
store the {{Integer.MAX_VALUE}}?
3. nit: {{Journal.LOG.info(logMsg.toString());}}: the {{toString}} is not
necessary?
4. Why we need {{getJournaledEditsCache()}} in {{JournalTestUtil}}? can we
replace it with {{journal.getJournaledEditsCache();}}?
5. nit: Can we also update {{JournaledEditsCache}} to add an empty line
between the Java doc and method parameters?
6. nit: in {{JournalTestUtil}}, maybe remove the extra empty line after
{{package org.apache.hadoop.hdfs.qjournal.server;}}?
7. nit: in Javadoc for {{JournalTestUtil#corruptJournaledEditsCache()}}, change
"simulated" to "simulate", and "transaction idea" to "transaction ID".
> [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove
> unnecessary dummy sync
> --
>
> Key: HDFS-13610
> URL: https://issues.apache.org/jira/browse/HDFS-13610
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, journal-node, namenode
>Reporter: Erik Krogen
>Assignee: Erik Krogen
>Priority: Major
> Attachments: HDFS-13610-HDFS-12943.000.patch,
> HDFS-13610-HDFS-12943.001.patch, HDFS-13610-HDFS-12943.002.patch,
> HDFS-13610-HDFS-12943.003.patch
>
>
> See HDFS-13150 for full design.
> This JIRA is targeted at cleanup tasks:
> * Add in integration testing. We can expand {{TestStandbyInProgressTail}}
> * Documentation in HDFSHighAvailabilityWithQJM
> * Remove the dummy sync added as part of HDFS-10519; it is unnecessary since
> now in-progress tailing does not rely on the JN committedTxnId
> A few bugs are also fixed:
> * Due to changes in HDFS-13609 to enable use of the RPC mechanism whenever
> inProgressOK is true, there were codepaths which would use the RPC mechanism
> even when dfs.ha.tail-edits.in-progress was false, meaning that the JNs did
> not enable the cache. Update the QJM logic to only use selectRpcInputStreams
> if this config is true.
> * Fix a false error logged when the layout version changes
> * Fix the logging when a layout version change occurs to avoid printing out a
> placeholder value (Integer.MAX_VALUE)
--
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-13610) [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove unnecessary dummy sync
[
https://issues.apache.org/jira/browse/HDFS-13610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16530191#comment-16530191
]
Chao Sun edited comment on HDFS-13610 at 7/2/18 5:13 PM:
-
Thanks [~xkrogen]! A few of comments (mostly nits):
1. In {{JournaledEditsCache}}, do we need the check: {{lowestTxnId < 0}}?
seems the cache is already initialized in constructor.
2. nit: Can we define a static variable such as {{INVALID_LAYOUT_VERSION}} to
store the {{Integer.MAX_VALUE}}?
3. nit: {{Journal.LOG.info(logMsg.toString());}}: the {{toString}} is not
necessary?
4. Why we need {{getJournaledEditsCache()}} in {{JournalTestUtil}}? can we
replace it with {{journal.getJournaledEditsCache();}}?
5. nit: Can we also update {{JournaledEditsCache}} to add an empty line
between the Java doc and method parameters?
6. nit: in {{JournalTestUtil}}, maybe remove the extra empty line after
{{package org.apache.hadoop.hdfs.qjournal.server;}}?
7. nit: in Javadoc for {{JournalTestUtil#corruptJournaledEditsCache()}}, change
"simulated" to "simulate", and "transaction idea" to "transaction ID".
was (Author: csun):
Thanks [~xkrogen]! A few of comments (mostly nits):
1. In {{JournaledEditsCache}}, do we need the check: {{lowestTxnId < 0}}? seems
the cache is already initialized in constructor.
2. nit: Can we define a static variable such as {{INVALID_LAYOUT_VERSION}} to
store the {{Integer.MAX_VALUE}}?
3. nit: {{Journal.LOG.info(logMsg.toString());}}: the {{toString}} is not
necessary?
4. Why we need {{getJournaledEditsCache()}} in {{JournalTestUtil}}? can we
replace it with {{journal.getJournaledEditsCache();}}?
5. nit: Can we also update {{JournaledEditsCache}} to add an empty line between
the Java doc and method parameters?
6. nit: in {{JournalTestUtil}}, maybe remove the extra empty line after
{{package org.apache.hadoop.hdfs.qjournal.server;}}?
> [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove
> unnecessary dummy sync
> --
>
> Key: HDFS-13610
> URL: https://issues.apache.org/jira/browse/HDFS-13610
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, journal-node, namenode
>Reporter: Erik Krogen
>Assignee: Erik Krogen
>Priority: Major
> Attachments: HDFS-13610-HDFS-12943.000.patch,
> HDFS-13610-HDFS-12943.001.patch, HDFS-13610-HDFS-12943.002.patch,
> HDFS-13610-HDFS-12943.003.patch
>
>
> See HDFS-13150 for full design.
> This JIRA is targeted at cleanup tasks:
> * Add in integration testing. We can expand {{TestStandbyInProgressTail}}
> * Documentation in HDFSHighAvailabilityWithQJM
> * Remove the dummy sync added as part of HDFS-10519; it is unnecessary since
> now in-progress tailing does not rely on the JN committedTxnId
> A few bugs are also fixed:
> * Due to changes in HDFS-13609 to enable use of the RPC mechanism whenever
> inProgressOK is true, there were codepaths which would use the RPC mechanism
> even when dfs.ha.tail-edits.in-progress was false, meaning that the JNs did
> not enable the cache. Update the QJM logic to only use selectRpcInputStreams
> if this config is true.
> * Fix a false error logged when the layout version changes
> * Fix the logging when a layout version change occurs to avoid printing out a
> placeholder value (Integer.MAX_VALUE)
--
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-13610) [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove unnecessary dummy sync
[
https://issues.apache.org/jira/browse/HDFS-13610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16526153#comment-16526153
]
Erik Krogen edited comment on HDFS-13610 at 6/28/18 10:05 AM:
--
Thanks for the review [~linyiqun]!
* Done, good idea.
* I updated these docs to mention the standby read feature. I also reduced the
width to make it more closely match the rest of this file (although there is
not much consistency...)
* Whitespace issue fixed
Uploaded v003. [~shv] or [~vagarychen], want to take a look as well?
was (Author: xkrogen):
Thanks for the review [~linyiqun]!
* Done, good idea.
* I updated these docs to mention the standby read feature. I also reduced the
width to make it more closely match the rest of this file (although there is
not much consistency...)
* Whitespace issue fixed
Uploaded v003. [~shv] or [~vagarychen], want to take a look?
> [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove
> unnecessary dummy sync
> --
>
> Key: HDFS-13610
> URL: https://issues.apache.org/jira/browse/HDFS-13610
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, journal-node, namenode
>Reporter: Erik Krogen
>Assignee: Erik Krogen
>Priority: Major
> Attachments: HDFS-13610-HDFS-12943.000.patch,
> HDFS-13610-HDFS-12943.001.patch, HDFS-13610-HDFS-12943.002.patch,
> HDFS-13610-HDFS-12943.003.patch
>
>
> See HDFS-13150 for full design.
> This JIRA is targeted at cleanup tasks:
> * Add in integration testing. We can expand {{TestStandbyInProgressTail}}
> * Documentation in HDFSHighAvailabilityWithQJM
> * Remove the dummy sync added as part of HDFS-10519; it is unnecessary since
> now in-progress tailing does not rely as heavily on the JN committedTxnId
--
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-13610) [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove unnecessary dummy sync
[
https://issues.apache.org/jira/browse/HDFS-13610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16526036#comment-16526036
]
Yiqun Lin edited comment on HDFS-13610 at 6/28/18 7:45 AM:
---
Thanks for updating the patch, [~xkrogen]! Two comments from me:
* For the {{TestStandbyInProgressTail#waitForFileInfo}}, I will prefer to pass
a file name list and do the removal in {{GenericTestUtils.waitFor}} once
getFileInfo returns value info. We don't need to request for all the files
every time.
* In the doc, can we mention the mainly use case of In-Progress Edit Log
Tailing? I mean we can document Standby Read there. This is why we make SBN to
have more up-to-date namespace info.
* Would you also fix whitespace issue that reported?
was (Author: linyiqun):
Thanks for updating the patch, [~xkrogen]! Two comments from me:
* For the {{TestStandbyInProgressTail#waitForFileInfo}}, I will prefer to pass
a file name list and do the removal in {{GenericTestUtils.waitFor}} once
getFileInfo returns value info. We don't need to request for all the files
every time.
* In the doc, can we mention the mainly use case of In-Progress Edit Log
Tailing? I mean we can document Standby Read there. This is why we make SBN to
have more up-to-date namespace info.
> [Edit Tail Fast Path Pt 4] Cleanup: integration test, documentation, remove
> unnecessary dummy sync
> --
>
> Key: HDFS-13610
> URL: https://issues.apache.org/jira/browse/HDFS-13610
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: ha, journal-node, namenode
>Reporter: Erik Krogen
>Assignee: Erik Krogen
>Priority: Major
> Attachments: HDFS-13610-HDFS-12943.000.patch,
> HDFS-13610-HDFS-12943.001.patch, HDFS-13610-HDFS-12943.002.patch
>
>
> See HDFS-13150 for full design.
> This JIRA is targeted at cleanup tasks:
> * Add in integration testing. We can expand {{TestStandbyInProgressTail}}
> * Documentation in HDFSHighAvailabilityWithQJM
> * Remove the dummy sync added as part of HDFS-10519; it is unnecessary since
> now in-progress tailing does not rely as heavily on the JN committedTxnId
--
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
