leventov commented on a change in pull request #7595: Optimize overshadowed
segments computation
URL: https://github.com/apache/incubator-druid/pull/7595#discussion_r290832092
##########
File path:
server/src/main/java/org/apache/druid/metadata/SQLMetadataSegmentManager.java
##########
@@ -461,18 +462,22 @@ public boolean removeDataSource(final String dataSource)
return true;
}
+ /**
+ * This method does not update {@code dataSourcesSnapshot}, see the comments
in {@code doPoll()} about
+ * snapshot update. The segment removal will be reflected after next poll
cyccle runs.
+ */
@Override
- public boolean removeSegment(String dataSourceName, final String segmentId)
+ public boolean removeSegment(String dataSourceName, final String identifier)
{
- try {
- return removeSegmentFromTable(segmentId);
- }
- catch (Exception e) {
- log.error(e, e.toString());
- return false;
- }
+ final SegmentId segmentId = SegmentId.tryParse(dataSourceName, identifier);
Review comment:
You parse a `SegmentId` object and pass it to a method which essentially
just converts it back to string. If these methods should still exist, the
`public boolean removeSegment(SegmentId segmentId)` should delegate to this
one, not the other way around. But actually I think it's better to leave only
string-accepting method and force clients to call it with `SegmentId` by
calling `toString()` themselves because `removeSegment(SegmentId segmentId)`
just calling `toString()` and calling into `removeSegment(String)` is too
shallow and makes the false impression of "better quality" (because accepts a
non-string object).
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
