kfaraz commented on code in PR #18149:
URL: https://github.com/apache/druid/pull/18149#discussion_r2153458542
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java:
##########
@@ -223,30 +223,27 @@ public Response specGetAll(
.withDetailedState(theState.get().toString())
.withHealthy(theState.get().isHealthy());
}
- if (includeFull) {
- Optional<SupervisorSpec> theSpec =
manager.getSupervisorSpec(x);
- if (theSpec.isPresent()) {
- theBuilder.withSpec(theSpec.get())
-
.withDataSource(theSpec.get().getDataSources().stream().findFirst().orElse(null));
+ Optional<SupervisorSpec> theSpec =
manager.getSupervisorSpec(x);
+ if (theSpec.isPresent()) {
+
theBuilder.withDataSource(theSpec.get().getDataSources().stream().findFirst().orElse(null));
Review Comment:
We can avoid the repeated invocation of `theSpec.get()` by doing it once and
assigning it to a variable.
##########
server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java:
##########
@@ -632,4 +633,21 @@ List<Interval> getUnusedSegmentIntervals(
*/
boolean markSegmentAsUsed(SegmentId segmentId);
+ /**
+ * Validates the given supervisorId and given metadata to ensure
+ * that start/end metadata non-null implies supervisor ID is non-null.
+ */
+ static void validateDataSourceMetadata(
+ @Nullable final String supervisorId,
+ @Nullable final DataSourceMetadata startMetadata,
+ @Nullable final DataSourceMetadata endMetadata
+ )
+ {
+ if ((startMetadata == null && endMetadata != null) || (startMetadata !=
null && endMetadata == null)) {
+ throw InvalidInput.exception("start/end metadata pair must be either
null or non-null");
+ } else if (startMetadata != null && supervisorId == null) {
+ throw InvalidInput.exception(
+ "supervisorId cannot be null if startMetadata and endMetadata are
both non-null.");
Review Comment:
```suggestion
"'supervisorId' cannot be null if 'startMetadata' and
'endMetadata' are both non-null.");
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1029,8 +1029,9 @@ public void start()
catch (Exception e) {
if (!started) {
log.warn(
- "First initialization attempt failed for
SeekableStreamSupervisor[%s], starting retries...",
- supervisorId
+ "First initialization attempt failed for
SeekableStreamSupervisor[%s] for dataSource[%s], starting retries...",
Review Comment:
For consistency with the other logs:
```suggestion
"First initialization attempt failed for supervisor[%s],
dataSource[%s], starting retries...",
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/overlord/supervisor/SupervisorResource.java:
##########
@@ -223,30 +223,27 @@ public Response specGetAll(
.withDetailedState(theState.get().toString())
.withHealthy(theState.get().isHealthy());
}
- if (includeFull) {
- Optional<SupervisorSpec> theSpec =
manager.getSupervisorSpec(x);
- if (theSpec.isPresent()) {
- theBuilder.withSpec(theSpec.get())
-
.withDataSource(theSpec.get().getDataSources().stream().findFirst().orElse(null));
+ Optional<SupervisorSpec> theSpec =
manager.getSupervisorSpec(x);
Review Comment:
@jtuglu-netflix , could you please attach a screenshot (in the PR
description itself) of the web-console showing the result of the sys query
after this change?
##########
server/src/main/java/org/apache/druid/indexing/overlord/IndexerMetadataStorageCoordinator.java:
##########
@@ -632,4 +633,21 @@ List<Interval> getUnusedSegmentIntervals(
*/
boolean markSegmentAsUsed(SegmentId segmentId);
+ /**
+ * Validates the given supervisorId and given metadata to ensure
+ * that start/end metadata non-null implies supervisor ID is non-null.
+ */
+ static void validateDataSourceMetadata(
+ @Nullable final String supervisorId,
+ @Nullable final DataSourceMetadata startMetadata,
+ @Nullable final DataSourceMetadata endMetadata
+ )
+ {
+ if ((startMetadata == null && endMetadata != null) || (startMetadata !=
null && endMetadata == null)) {
+ throw InvalidInput.exception("start/end metadata pair must be either
null or non-null");
Review Comment:
Minor rephrase:
```suggestion
throw InvalidInput.exception("'startMetadata' and 'endMetadata' must
either both be null or both non-null");
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
