Repository navigation
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13825/ |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13826/ |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
the previous failures are due to timeout and failed to fetch remote repo, anyone wants to review this? |
Conf improvements There are two new features. 1. Allow users to set arbitrary akka configurations via spark conf. 2. Allow configuration to be printed in logs for diagnosis.
|
Hi, @kayousterhout , mind reviewing this? |
|
I won't have time to review this for a week or so |
|
sure, take your time I notified you just because you're the original reporter of the issue and I want to ensure I understand the issue correctly |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
rebased~ |
|
The problem I was describing in the JIRA is that all but the last 2 tests in this suite are designed to test the pool / fairness mechanisms (i.e., that the output of rootPool.getSortedTaskSetQueue() is correct), not the scheduler code. I think the right solution here is to move these tests into a separate test suite specifically for the pool stuff -- not to make these tests use the scheduler (since using the scheduler obfuscates where the actually error would be if the tests failed). |
|
Hi, @kayousterhout, you mean just split the test cases and move some of them to another file? I can do that...but do you think the test cases specific to the TaskSchedulerImpl class are also missed? (except the last two) I think the implementation in this patch is equivalent to the original one, as we assume the locality is Any and the assert statements are actually testing the results of rootPool.getSortedTaskSetQueue(), I think for fully test the critical code path of TaskSchedulerImpl, we need some test cases on the locality? |
|
We definitely don't have sufficient testing for TaskSchedulerImpl right now On Sun, Apr 13, 2014 at 2:57 PM, Nan Zhu [email protected] wrote:
|
|
Merged build triggered. |
|
QA tests have finished for PR 339 at commit
|
|
Test FAILed. |
|
QA tests have started for PR 339 at commit
|
|
QA tests have finished for PR 339 at commit
|
|
Test FAILed. |
|
QA tests have started for PR 339 at commit
|
I don't know where this message is coming from, and I don't see any recent changes to the |
|
QA tests have finished for PR 339 at commit
|
|
Test PASSed. |
|
@CodingCat @kayousterhout What's the status of this PR? It hasn't been updated in several months. |
|
@kayousterhout does this PR reflect what you had in mind in your description for SPARK-1143 |
|
@andrewor14 I need to add a design doc then? |
|
@CodingCat @andrewor14 This isn't quite what I had in mind; I'm going to submit a (simpler) PR for this in the next few hours. I think this one is more complicated than necessary. |
|
No need for design doc; I thought for a second you were refactoring |
|
Ok, since @kayousterhout is planning on submitting a simpler version of this, @CodingCat would you mind closing this PR? |
|
sure |
The current TaskSchedulerImplSuite includes some tests that are actually for the TaskSchedulerImpl, but many of the tests avoid using the TaskSchedulerImpl entirely, and actually test the pool and scheduling algorithm mechanisms. This commit separates the pool/scheduling algorithm tests into their own suite, simplifies those tests, and attempts to more helpfully comment the fair scheduler test. The pull request replaces apache#339.
The current TaskSchedulerImplSuite includes some tests that are actually for the TaskSchedulerImpl, but the remainder of the tests avoid using the TaskSchedulerImpl entirely, and actually test the pool and scheduling algorithm mechanisms. This commit separates the pool/scheduling algorithm tests into their own suite, and also simplifies those tests. The pull request replaces #339. Author: Kay Ousterhout <[email protected]> Closes #3967 from kayousterhout/SPARK-1143 and squashes the following commits: 8a898c4 [Kay Ousterhout] [SPARK-1143] Separate pool tests into their own suite.
Fix if issue in keyword contain case
### What changes were proposed in this pull request? Step 6a of task 87 (`sql/varka/plans/PLAN_TASK_87.md`, section 6): `VarkaMethodSizeBenchmark` gains its second arm and its committed files are regenerated with both forms in one table per rung, banded over ten runs. Stacked on apache#339 (step 5); the diff here is the last two commits once that merges. The two forms are named explicitly so the labels survive the default changing: `single epilogue`, the legacy emission (`methodByteBudget` 0), and `epilogue per group`, the emission under the budget at `HugeMethodLimit` - a group's methods set up only their group (apache#337), the epilogue is one method per group (apache#338), and the class is measured and regrouped (apache#339). `VarkaEmitBudget` is public so a benchmark outside the emitter's package can name the limit. The results, in M rows/s at 512 bits on the masked arm (the arm that reaches the single epilogue's crossing at thirteen outputs): | outputs | even batches | ragged batches | | ---: | :--- | :--- | | 4 | 441.9 -> 434.1 | 431.2 -> 442.8 | | 12 | 134.0 -> 143.1 | 135.2 -> 142.3 | | 13 | 120.9 -> 132.9 | 40.7 -> **131.7** | | 16 | 94.4 -> 106.3 | 32.9 -> 104.9 | | 60 | 24.1 -> 29.4 | 8.7 -> 28.9 | The cliff is gone: on ragged batches the per-group form reads within a few percent of its own even-batch rate at every rung, where the single form had lost three quarters of its rate from thirteen outputs. And from twelve outputs the per-group form is faster on even batches too, by 7% at twelve and 22% at sixty, which no prediction had: each loop method now sets up only its group's segments and literals, and that outweighs the driver's extra epilogue calls. The 128-bit file reads the same way (13 outputs ragged: 24.9 -> 44.0). Section 9.5 of the plan scores the registered predictions: 3 held and understated the gain, 4 fell on the fast side, 5 is superseded by the design; the default's rule - nowhere worse beyond its band on the masked arm at any rung, length or width, and on the null-free arm up to twelve outputs - is met, and the flip is step 6b. One lead for task 189: in this fork the single form's null-free rows at 128 bits from twelve outputs sit in the deoptimization cycle (0.3 to 0.7 M rows/s ragged) and the per-group form's do not (40 to 56). One fork proves nothing about a bimodal effect; the plan records it as a lead, not a property. ### Why are the changes needed? The plan's rule for flipping the default is a measurement, and this is it: both forms in one file, at both widths, with the band that says which differences mean anything. ### Does this PR introduce _any_ user-facing change? No. The default is unchanged in this PR. ### How was this patch tested? The benchmark is the test: `dev/varka_bench_regen.sh catalyst VarkaMethodSizeBenchmark` (pinned, load 0.57 at start, canary clean, invariants ok at both widths) and `dev/varka_bench_repeat.sh catalyst VarkaMethodSizeBenchmark 10 --band`. `dev/varka_quote_check.py` passes for every number quoted in the plan. The suites do not change. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5.1)
https://issues.apache.org/jira/browse/SPARK-1143
The current TaskSchedulerImplSuite actually bypasses the scheduling code (mainly resourceOffer())
In this patch, I removed the original dummy ResourceOffer and made the test case call taskScheduler.resourceOffer() directly,