capistrant commented on pull request #10287:
URL: https://github.com/apache/druid/pull/10287#issuecomment-756913585
> I like this log message from LogUsedSegments and have found it very useful:
>
> ```java
> log.info("Found [%,d] used segments.", params.getUsedSegments().size());
> ```
>
> It's nearly free, since `params.getUsedSegments()` is something that's
just passed into the method and doesn't need to be computed. Instead of a
config to disable the duty, how about about removing this block and leaving
everything else the same:
>
> ```java
> DataSourcesSnapshot dataSourcesSnapshot =
params.getDataSourcesSnapshot();
> for (DataSegment segment :
dataSourcesSnapshot.iterateAllUsedSegmentsInSnapshot()) {
> if (segment.getSize() < 0) {
> log.makeAlert("No size on a segment")
> .addData("segment", segment)
> .emit();
> }
> }
> ```
>
> I'm pretty sure this is ancient debugging code and we can safely get rid
of it.
>
> The other potentially expensive part is skipped if log level is INFO or
higher. (Which is the default, so it shouldn't be a problem.)
I think I like this idea the most considering everything we have learned
since I created the PR. I am going to amend the PR to remove the configuration
to skip the duty. Instead we will remove this legacy code and leave everything
else as is. Maybe in the future this duty will change and become expensive
resulting in having the ability to skip become beneficial. but lets not put the
cart ahead of the horse and over complicate things today.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
