Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-21621][Core] Reset numRecordsWritten after DiskBlockObjectWriter.commitAndGet called #18830

Closed
wants to merge 1 commit into from

Conversation

ConeyLiu
Copy link
Contributor

@ConeyLiu ConeyLiu commented Aug 3, 2017

What changes were proposed in this pull request?

We should reset numRecordsWritten to zero after DiskBlockObjectWriter.commitAndGet called.
Because when revertPartialWritesAndClose be called, we decrease the written records in ShuffleWriteMetrics . However, we decreased the written records to zero, this should be wrong, we should only decreased the number reords after the last commitAndGet called.

How was this patch tested?

Modified existing test.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@ConeyLiu
Copy link
Contributor Author

ConeyLiu commented Aug 3, 2017

You can see here L208, when we called 'revertPartialWritesAndClose', the written reocrds will decrease to 0.

@ConeyLiu
Copy link
Contributor Author

ConeyLiu commented Aug 3, 2017

@cloud-fan @vanzin Would you mind take a look? Thanks a lot.

@jerryshao
Copy link
Contributor

Nice catch, looks good to me.

Copy link
Contributor

@jiangxb1987 jiangxb1987 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jiangxb1987
Copy link
Contributor

ok to test

@ConeyLiu
Copy link
Contributor Author

ConeyLiu commented Aug 5, 2017

Thanks for reviewing. Hi @jiangxb1987, seems the test didn't triggered.

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Aug 7, 2017

Test build #80316 has finished for PR 18830 at commit d82401d.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@cloud-fan
Copy link
Contributor

LGTM, merging to master/2.2

asfgit pushed a commit that referenced this pull request Aug 7, 2017
…er.commitAndGet called

## What changes were proposed in this pull request?

We should reset numRecordsWritten to zero after DiskBlockObjectWriter.commitAndGet called.
Because when `revertPartialWritesAndClose` be called, we decrease the written records in `ShuffleWriteMetrics` . However, we decreased the written records to zero, this should be wrong, we should only decreased the number reords after the last `commitAndGet` called.

## How was this patch tested?
Modified existing test.

Please review http://spark.apache.org/contributing.html before opening a pull request.

Author: Xianyang Liu <xianyang.liu@intel.com>

Closes #18830 from ConeyLiu/DiskBlockObjectWriter.

(cherry picked from commit 534a063)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
@asfgit asfgit closed this in 534a063 Aug 7, 2017
@ConeyLiu
Copy link
Contributor Author

ConeyLiu commented Aug 7, 2017

Thank you all.

@ConeyLiu ConeyLiu deleted the DiskBlockObjectWriter branch August 7, 2017 10:38
MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
…er.commitAndGet called

## What changes were proposed in this pull request?

We should reset numRecordsWritten to zero after DiskBlockObjectWriter.commitAndGet called.
Because when `revertPartialWritesAndClose` be called, we decrease the written records in `ShuffleWriteMetrics` . However, we decreased the written records to zero, this should be wrong, we should only decreased the number reords after the last `commitAndGet` called.

## How was this patch tested?
Modified existing test.

Please review http://spark.apache.org/contributing.html before opening a pull request.

Author: Xianyang Liu <xianyang.liu@intel.com>

Closes apache#18830 from ConeyLiu/DiskBlockObjectWriter.

(cherry picked from commit 534a063)
Signed-off-by: Wenchen Fan <wenchen@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants