Skip to content

Conversation

@srowen
Copy link
Member

@srowen srowen commented Oct 1, 2016

What changes were proposed in this pull request?

Return Iterator of applications internally in history server, for consistency and performance. See #15248 for some back-story.

The code called by and calling HistoryServer.getApplicationList wants an Iterator, but this method materializes an Iterable, which potentially causes a performance problem. It's simpler too to make this internal method also pass through an Iterator.

How was this patch tested?

Existing tests.

@srowen
Copy link
Member Author

srowen commented Oct 1, 2016

CC @wgtmac and @ajbozarth for a look, in case I'm missing something.

@SparkQA
Copy link

SparkQA commented Oct 1, 2016

Test build #66230 has finished for PR 15321 at commit 14fb9a0.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@wgtmac
Copy link
Member

wgtmac commented Oct 2, 2016

LGTM

@ajbozarth
Copy link
Member

ajbozarth commented Oct 2, 2016

Looks like some tests need to be updated, but what's here LGTM

@SparkQA
Copy link

SparkQA commented Oct 2, 2016

Test build #66242 has finished for PR 15321 at commit d90c38c.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 2, 2016

Test build #66243 has finished for PR 15321 at commit 315407c.

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

@ajbozarth
Copy link
Member

LGTM

@SparkQA
Copy link

SparkQA commented Oct 2, 2016

Test build #66245 has finished for PR 15321 at commit e385be8.

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

@srowen
Copy link
Member Author

srowen commented Oct 4, 2016

Merged to master

@asfgit asfgit closed this in 8e8de00 Oct 4, 2016
@srowen srowen deleted the SPARK-17671 branch October 4, 2016 10:02
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.

4 participants