Skip to content

Conversation

@tdas
Copy link
Contributor

@tdas tdas commented Aug 9, 2017

Same PR as #18799 but for branch 2.2. Main discussion the other PR.

When I was investigating a flaky test, I realized that many places don't check the return value of HDFSMetadataLog.get(batchId: Long): Option[T]. When a batch is supposed to be there, the caller just ignores None rather than throwing an error. If some bug causes a query doesn't generate a batch metadata file, this behavior will hide it and allow the query continuing to run and finally delete metadata logs and make it hard to debug.

This PR ensures that places calling HDFSMetadataLog.get always check the return value.

Jenkins

… return value

When I was investigating a flaky test, I realized that many places don't check the return value of `HDFSMetadataLog.get(batchId: Long): Option[T]`. When a batch is supposed to be there, the caller just ignores None rather than throwing an error. If some bug causes a query doesn't generate a batch metadata file, this behavior will hide it and allow the query continuing to run and finally delete metadata logs and make it hard to debug.

This PR ensures that places calling HDFSMetadataLog.get always check the return value.

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#18799 from zsxwing/SPARK-21596.
@zsxwing
Copy link
Member

zsxwing commented Aug 9, 2017

LGTM

@SparkQA
Copy link

SparkQA commented Aug 9, 2017

Test build #80436 has finished for PR 18890 at commit 532529a.

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

@zsxwing
Copy link
Member

zsxwing commented Aug 9, 2017

Thanks! Merging to branch-2.2.

asfgit pushed a commit that referenced this pull request Aug 9, 2017
… return value

Same PR as #18799 but for branch 2.2. Main discussion the other PR.
--------

When I was investigating a flaky test, I realized that many places don't check the return value of `HDFSMetadataLog.get(batchId: Long): Option[T]`. When a batch is supposed to be there, the caller just ignores None rather than throwing an error. If some bug causes a query doesn't generate a batch metadata file, this behavior will hide it and allow the query continuing to run and finally delete metadata logs and make it hard to debug.

This PR ensures that places calling HDFSMetadataLog.get always check the return value.

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes #18890 from tdas/SPARK-21596-2.2.
@zsxwing
Copy link
Member

zsxwing commented Aug 9, 2017

Merged. Could you close the PR?

@tdas
Copy link
Contributor Author

tdas commented Aug 9, 2017

Thanks @zsxwing

@tdas tdas closed this Aug 9, 2017
MatthewRBruce pushed a commit to Shopify/spark that referenced this pull request Jul 31, 2018
… return value

Same PR as apache#18799 but for branch 2.2. Main discussion the other PR.
--------

When I was investigating a flaky test, I realized that many places don't check the return value of `HDFSMetadataLog.get(batchId: Long): Option[T]`. When a batch is supposed to be there, the caller just ignores None rather than throwing an error. If some bug causes a query doesn't generate a batch metadata file, this behavior will hide it and allow the query continuing to run and finally delete metadata logs and make it hard to debug.

This PR ensures that places calling HDFSMetadataLog.get always check the return value.

Jenkins

Author: Shixiong Zhu <[email protected]>

Closes apache#18890 from tdas/SPARK-21596-2.2.
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.

3 participants