Jackie-Jiang commented on code in PR #19998:
URL: https://github.com/apache/pinot/pull/19998#discussion_r4416504527
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/SegmentMetadataImpl.java:
##########
@@ -483,21 +621,136 @@ public String getEndOffset() {
@Override
public TreeMap<String, ColumnMetadata> getColumnMetadataMap() {
- return _columnMetadataMap;
+ if (_mappedStore == null) {
+ return _columnMetadataMap;
+ }
+ // Compatibility API for metadata/admin callers. The serving path uses
column names and direct lookup instead.
+ TreeMap<String, ColumnMetadata> result = new TreeMap<>();
+ for (String column : getPhysicalColumnNames()) {
+ result.put(column, getColumnMetadataFor(column));
+ }
+ result.putAll(_virtualColumnMetadata);
+ return result;
+ }
+
+ /// Returns the physical column names without materializing a metadata map
or Schema on mapped segments.
+ public Set<String> getPhysicalColumnNames() {
+ return _mappedStore != null ? _physicalColumnNames :
_schema.getPhysicalColumnNames();
+ }
+
+ @Override
+ public NavigableSet<String> getAllColumns() {
+ if (_mappedStore == null) {
+ return getSchema().getColumnNames();
+ }
+ NavigableSet<String> columns = _mappedAllColumns;
+ if (columns == null) {
+ NavigableSet<String> allColumns = new
TreeSet<>(getPhysicalColumnNames());
+ allColumns.addAll(_virtualColumnMetadata.keySet());
+ columns = Collections.unmodifiableNavigableSet(allColumns);
+ _mappedAllColumns = columns;
+ }
+ return columns;
+ }
+
+ /// Indicates whether the immutable segment is reading its metadata from the
binary mapping.
+ public boolean isMapped() {
+ return _mappedStore != null;
+ }
+
+ /// Returns metadata that remains readable after the mapped segment is
released. Eager metadata already owns its
+ /// column data; a mapped segment needs an eager copy with its serving-time
column overlays and ZK timestamps.
+ public SegmentMetadataImpl detachedCopy()
+ throws IOException, ConfigurationException {
Review Comment:
[P3] Remove the obsolete checked exceptions from detachedCopy()
The new implementation materializes the existing in-memory/mapped state and
no longer performs checked I/O or configuration parsing. The
IOException/ConfigurationException declaration is left over from reopening
_indexDir, and forces HelixInstanceDataManager.detachMappedMetadata() to retain
an unnecessary try/catch and IllegalStateException wrapper. Please remove these
declarations and simplify that caller. This is a nonblocking cleanup; the
in-memory snapshot fixes the reload race.
--
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]
