[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[
https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14527935#comment-14527935
]
TezQA commented on TEZ-2392:
{color:green}+1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12730353/TEZ-2392.3.patch
against master revision 210619a.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 5 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:green}+1 core tests{color}. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/620//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/620//console
This message is automatically generated.
Have all readers throw an Exception on incorrect next() usage
-
Key: TEZ-2392
URL: https://issues.apache.org/jira/browse/TEZ-2392
Project: Apache Tez
Issue Type: Improvement
Reporter: Siddharth Seth
Assignee: Rajesh Balamohan
Priority: Critical
Attachments: TEZ-2392.1.patch, TEZ-2392.2.patch, TEZ-2392.3.patch
Follow up from TEZ-2348.
Marking as critical since this is a behaviour change, and we should get it in
early.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[ https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14527538#comment-14527538 ] Siddharth Seth commented on TEZ-2392: - Patch looks good to me - ValuesIterator has the check in the main path, but I'm not sure that can be avoided. +1 In case of the MRReaders, the check being after recordReader.next leaves this open to an exception from user code. Should just document this (in MRInput.getReader()) - An exception will be thrown if next() is invoked after false, either from the framework or from the underlying InputFormat. Have all readers throw an Exception on incorrect next() usage - Key: TEZ-2392 URL: https://issues.apache.org/jira/browse/TEZ-2392 Project: Apache Tez Issue Type: Improvement Reporter: Siddharth Seth Assignee: Rajesh Balamohan Priority: Critical Attachments: TEZ-2392.1.patch, TEZ-2392.2.patch Follow up from TEZ-2348. Marking as critical since this is a behaviour change, and we should get it in early. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[ https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14524433#comment-14524433 ] Rajesh Balamohan commented on TEZ-2392: --- Why is the hasCompletedProcessing check being done after calling recordReader.next() ? Shouldn't be called before? Wanted to keep the regular path unaffected with this change (without branches). From a thread safety point of view, is completedProcessing protected properly? Thread safety was not guaranteed for KeyValueReader/KeyValuesReader. For e.g, OrderedGroupedKeyValuesReader internally makes use of ValuesIterator which does not guarantee thread safety. Have all readers throw an Exception on incorrect next() usage - Key: TEZ-2392 URL: https://issues.apache.org/jira/browse/TEZ-2392 Project: Apache Tez Issue Type: Improvement Reporter: Siddharth Seth Assignee: Rajesh Balamohan Priority: Critical Attachments: TEZ-2392.1.patch, TEZ-2392.2.patch Follow up from TEZ-2348. Marking as critical since this is a behaviour change, and we should get it in early. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[ https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14523638#comment-14523638 ] Hitesh Shah commented on TEZ-2392: -- From a thread safety point of view, is completedProcessing protected properly? Have all readers throw an Exception on incorrect next() usage - Key: TEZ-2392 URL: https://issues.apache.org/jira/browse/TEZ-2392 Project: Apache Tez Issue Type: Improvement Reporter: Siddharth Seth Assignee: Rajesh Balamohan Priority: Critical Attachments: TEZ-2392.1.patch, TEZ-2392.2.patch Follow up from TEZ-2348. Marking as critical since this is a behaviour change, and we should get it in early. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[
https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14523636#comment-14523636
]
Hitesh Shah commented on TEZ-2392:
--
Not sure I understand this change:
{code}
boolean hasNext = recordReader.next(key, value);
if (hasNext) {
inputRecordCounter.increment(1);
} else {
hasCompletedProcessing();
completedProcessing = true;
}
{code}
Why is the hasCompletedProcessing check being done after calling
recordReader.next() ? Shouldn't be called before? i.e.
{code}
hasCompletedProcessing();
boolean hasNext = recordReader.next(key, value);
if (hasNext) {
inputRecordCounter.increment(1);
} else {
completedProcessing = true;
}
{code}
Have all readers throw an Exception on incorrect next() usage
-
Key: TEZ-2392
URL: https://issues.apache.org/jira/browse/TEZ-2392
Project: Apache Tez
Issue Type: Improvement
Reporter: Siddharth Seth
Assignee: Rajesh Balamohan
Priority: Critical
Attachments: TEZ-2392.1.patch, TEZ-2392.2.patch
Follow up from TEZ-2348.
Marking as critical since this is a behaviour change, and we should get it in
early.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (TEZ-2392) Have all readers throw an Exception on incorrect next() usage
[
https://issues.apache.org/jira/browse/TEZ-2392?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14522930#comment-14522930
]
TezQA commented on TEZ-2392:
{color:red}-1 overall{color}. Here are the results of testing the latest
attachment
http://issues.apache.org/jira/secure/attachment/12729727/TEZ-2392.1.patch
against master revision 96efae0.
{color:green}+1 @author{color}. The patch does not contain any @author
tags.
{color:green}+1 tests included{color}. The patch appears to include 4 new
or modified test files.
{color:green}+1 javac{color}. The applied patch does not increase the
total number of javac compiler warnings.
{color:green}+1 javadoc{color}. There were no new javadoc warning messages.
{color:green}+1 findbugs{color}. The patch does not introduce any new
Findbugs (version 2.0.3) warnings.
{color:green}+1 release audit{color}. The applied patch does not increase
the total number of release audit warnings.
{color:red}-1 core tests{color}. The patch failed these unit tests in :
org.apache.tez.runtime.library.common.TestValuesIterator
Test results: https://builds.apache.org/job/PreCommit-TEZ-Build/598//testReport/
Console output: https://builds.apache.org/job/PreCommit-TEZ-Build/598//console
This message is automatically generated.
Have all readers throw an Exception on incorrect next() usage
-
Key: TEZ-2392
URL: https://issues.apache.org/jira/browse/TEZ-2392
Project: Apache Tez
Issue Type: Improvement
Reporter: Siddharth Seth
Assignee: Rajesh Balamohan
Priority: Critical
Attachments: TEZ-2392.1.patch
Follow up from TEZ-2348.
Marking as critical since this is a behaviour change, and we should get it in
early.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
