kfaraz commented on code in PR #17785: URL: https://github.com/apache/druid/pull/17785#discussion_r2013070438
########## server/src/main/java/org/apache/druid/metadata/segment/cache/SegmentRecord.java: ########## @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.metadata.segment.cache; + +import org.apache.druid.java.util.common.DateTimes; +import org.apache.druid.java.util.common.logger.Logger; +import org.apache.druid.timeline.SegmentId; +import org.joda.time.DateTime; + +import javax.annotation.Nullable; +import java.sql.ResultSet; + +/** + * Represents a single record in the druid_segments table. + */ +class SegmentRecord +{ + private static final Logger log = new Logger(SegmentRecord.class); + + private final SegmentId segmentId; + private final boolean isUsed; + private final DateTime lastUpdatedTime; + + SegmentRecord(SegmentId segmentId, boolean isUsed, DateTime lastUpdatedTime) + { + this.segmentId = segmentId; + this.isUsed = isUsed; + this.lastUpdatedTime = lastUpdatedTime; + } + + public SegmentId getSegmentId() + { + return segmentId; + } + + public boolean isUsed() + { + return isUsed; + } + + public DateTime getLastUpdatedTime() + { + return lastUpdatedTime; + } + + /** + * Creates a SegmentRecord from the given result set. + * + * @return null if an error occurred while reading the record. + */ + @Nullable + static SegmentRecord fromResultSet(ResultSet r) + { + String serializedId = null; + String dataSource = null; + try { + serializedId = r.getString("id"); + dataSource = r.getString("dataSource"); + + final DateTime lastUpdatedTime = nullSafeDate(r.getString( + "used_status_last_updated")); + + final SegmentId segmentId = SegmentId.tryParse(dataSource, serializedId); + if (segmentId == null) { + log.noStackTrace().error( + "Could not parse Segment ID[%s] of datasource[%s]", + serializedId, dataSource + ); + return null; + } else { + return new SegmentRecord(segmentId, true, lastUpdatedTime); + } + } + catch (Exception e) { + log.noStackTrace().error( Review Comment: > When might this happen? Does it mean we will ignore segments that we can't read? That would be worrying, because Exception is very broad. Is there some kind of scenario you have in mind where a failure is possible? This can happen in the rare case when the segment payload has been tampered or some other column was not parseable. It is not frequent but it can happen, as I only recently encountered this in a prod DB. We are not throwing an error here so that the processing can continue with the rest of the segments. Even though this segment is ignored here, it actually increments a `skippedCount` in the code where this class is used and we raise an alert if `skippedCount > 0`. The log here is just to allow the operator to go through Overlord logs and see which segment IDs failed and why. -- 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]
