You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ege-st (via GitHub)" <gi...@apache.org> on 2024/02/01 15:04:50 UTC
Re: [PR] Enhancing metadata API to return upsert partition to primary key count map for both controller and server APIs [pinot]
ege-st commented on code in PR #12334:
URL: https://github.com/apache/pinot/pull/12334#discussion_r1474617679
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/models/DummyTableUpsertMetadataManager.java:
##########
@@ -72,6 +73,11 @@ public PartitionUpsertMetadataManager getOrCreatePartitionManager(int partitionI
public void stop() {
}
+ @Override
+ public Map<Integer, Long> getPartitionToPrimaryKeyCount() {
Review Comment:
Do we have any integration tests that call this mock?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/upsert/ConcurrentMapTableUpsertMetadataManager.java:
##########
@@ -45,6 +46,15 @@ public void stop() {
}
}
+ @Override
+ public Map<Integer, Long> getPartitionToPrimaryKeyCount() {
Review Comment:
Do we have unit tests that hit this method?
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -283,9 +284,23 @@ public String getSegmentMetadata(
}
}
+ // fetch partition to primary key count for realtime tables that have upsert enabled
+ Map<Integer, Long> upsertPartitionToPrimaryKeyCountMap = new HashMap<>();
+ if (tableDataManager instanceof RealtimeTableDataManager) {
+ RealtimeTableDataManager realtimeTableDataManager = (RealtimeTableDataManager) tableDataManager;
+ upsertPartitionToPrimaryKeyCountMap = realtimeTableDataManager.getUpsertPartitionToPrimaryKeyCount();
Review Comment:
My concern here is: how will a user tell the difference between a table that does _not_ have Upsert and a table that _has_ Upsert but no rows (is empty)?
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -283,9 +284,23 @@ public String getSegmentMetadata(
}
}
+ // fetch partition to primary key count for realtime tables that have upsert enabled
+ Map<Integer, Long> upsertPartitionToPrimaryKeyCountMap = new HashMap<>();
+ if (tableDataManager instanceof RealtimeTableDataManager) {
+ RealtimeTableDataManager realtimeTableDataManager = (RealtimeTableDataManager) tableDataManager;
+ upsertPartitionToPrimaryKeyCountMap = realtimeTableDataManager.getUpsertPartitionToPrimaryKeyCount();
Review Comment:
What will the value of this map be if the table does not have upsert enabled? If I understand correctly, this will be `0` if Upsert is not enabled on a table?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org