surekhasaharan commented on a change in pull request #6094: Introduce
SystemSchema tables (#5989)
URL: https://github.com/apache/incubator-druid/pull/6094#discussion_r221496420
##########
File path:
sql/src/main/java/org/apache/druid/sql/calcite/schema/DruidSchema.java
##########
@@ -320,25 +322,32 @@ public void awaitInitialization() throws
InterruptedException
private void addSegment(final DruidServerMetadata server, final DataSegment
segment)
{
synchronized (lock) {
- final Map<DataSegment, RowSignature> knownSegments =
segmentSignatures.get(segment.getDataSource());
+ final Map<DataSegment, SegmentMetadataHolder> knownSegments =
segmentMetadataInfo.get(segment.getDataSource());
if (knownSegments == null || !knownSegments.containsKey(segment)) {
+ final long isRealtime = server.segmentReplicatable() ? 0 : 1;
+ final long isPublished =
server.getType().toString().equals(ServerType.HISTORICAL.toString()) ? 1 : 0;
+ SegmentMetadataHolder holder = new SegmentMetadataHolder(null,
isPublished, 1, isRealtime, 1, null);
// Unknown segment.
- setSegmentSignature(segment, null);
+ setSegmentSignature(segment, holder);
segmentsNeedingRefresh.add(segment);
-
if (!server.segmentReplicatable()) {
log.debug("Added new mutable segment[%s].", segment.getIdentifier());
mutableSegments.add(segment);
} else {
log.debug("Added new immutable segment[%s].",
segment.getIdentifier());
}
- } else if (server.segmentReplicatable()) {
- // If a segment shows up on a replicatable (historical) server at any
point, then it must be immutable,
- // even if it's also available on non-replicatable (realtime) servers.
- mutableSegments.remove(segment);
- log.debug("Segment[%s] has become immutable.",
segment.getIdentifier());
+ } else {
+ if (knownSegments != null && knownSegments.containsKey(segment)) {
Review comment:
removed the null check on `knownSegments`
> and this can cause the race condition if we remove `lock` of this class
But I am not removing the `lock` here, so don't understand where is the race
condition then ?
> Why not use `computeIfAbsent()`?
It seems `computeIfAbsent()` also calls `doGet()`. Why do you think, this
will not cause race condition if we remove the `lock`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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]
