[b456377401] Implement Spark History discovery #1064
Conversation
dominik-burdzy
requested review from
shevek-google and
vladislav-sidorovich
as code owners
Code Coverage Report
|
dominik-burdzy
force-pushed
the
feature/b456377401/discover-spark-history
branch
2 times, most recently
from
0373d4b to
0ce3ab6
Compare
| private static final Logger logger = LoggerFactory.getLogger(SparkHistoryDiscoveryService.class); | ||
|
|
||
| private static final List<String> CANDIDATE_PATHS = | ||
| ImmutableList.of("spark3history", "sparkhistory"); |
There was a problem hiding this comment.
It's nice. In general we provide an option to users to override or provide the additional possible values as CLI parameters.
I think it can be useful here as well to add an additional parameter that users may use to specify their custom name.
There was a problem hiding this comment.
In this case we also should create a ticket to update our public documentation :)
There was a problem hiding this comment.
Good idea. I added customCadidatePaths to method params. I will pass user argument there in the task I will create in the next PR.
...ation/dumper/application/dumper/connector/cloudera/manager/SparkHistoryDiscoveryService.java
Outdated
Show resolved
Hide resolved
...n/dumper/application/dumper/connector/cloudera/manager/SparkHistoryDiscoveryServiceTest.java
Show resolved
Hide resolved
| import com.fasterxml.jackson.annotation.JsonProperty; | ||
|
|
||
| @JsonIgnoreProperties(ignoreUnknown = true) | ||
| public class ApiConfigDTO { |
There was a problem hiding this comment.
We need to standardize our treatment of acronyms in class names. To maintain consistency, we should choose between strict PascalCase (ApiRoleDto) or preserving the acronym (APIRoleDTO). Let's pick one to avoid inconsistent naming in the future.
There was a problem hiding this comment.
Good idea. I will refactor all of them to PascalCase.
|
|
||
| /** | ||
| * Discovers the active Spark History Server URL. Probes Spark 3 and Spark 2 endpoints to see | ||
| * which one is alive. |
There was a problem hiding this comment.
Can it be a scenario where both Spark 3 and Spark 2 are alive?
There was a problem hiding this comment.
I would expect it is a really rare case, but as you mentioned it I think it could happen when user modernizes the env and move some jobs from Spark2 to Spark3. I reimplemented it to return all possible reachable urls.
| * Discovers the active Spark History Server URL. Probes Spark 3 and Spark 2 endpoints to see | ||
| * which one is alive. | ||
| */ | ||
| public Optional<String> resolveUrl(String clusterName, CloseableHttpClient knoxClient) { |
There was a problem hiding this comment.
I couldn't find any active references to this method. Is it intended for use in a future CL?
There was a problem hiding this comment.
Yes, exactly. It will be used by a task parsing Spark event logs and extracting spark application metadata into jsonl.
dominik-burdzy
force-pushed
the
feature/b456377401/discover-spark-history
branch
from
0ce3ab6 to
24fbabf
Compare
Add SparkHistoryDiscoveryService and supporting DTOs to automatically resolve Spark History Server URLs via Knox by probing Spark 2 and Spark 3 endpoints.
dominik-burdzy
force-pushed
the
feature/b456377401/discover-spark-history
branch
from
24fbabf to
81c2257
Compare
3 participants
Add SparkHistoryDiscoveryService and supporting DTOs to automatically resolve Spark History Server URLs via Knox by probing Spark 2 and Spark 3 endpoints.