Jackie-Jiang opened a new pull request, #19792: URL: https://github.com/apache/pinot/pull/19792
## Summary Offline upsert tables (#17789) rely on partition based segment assignment to keep all segments of a partition on the same server: the upsert metadata of a server only covers the segments it hosts, so a partition spread across servers answers with duplicate primary keys. The validator only required a `segmentPartitionConfig`, which does not control placement: - Without an `instanceAssignmentConfigMap.OFFLINE`, the default instance partitions are one replica group with one partition, so `BalancedNumSegmentAssignmentStrategy` spreads a partition across every server. - With explicit partitions but no `numInstancesPerPartition`, the slot defaults to every instance of the replica group, and `ReplicaGroupSegmentAssignmentStrategy` picks the least loaded instance within it, again spreading the partition. - The legacy `replicaGroupStrategyConfig` only takes effect together with a replica-group `segmentAssignmentConfigMap` entry, and passes `numInstancesPerPartition` through unchanged. Realtime upsert and dedup tables are not affected: `RealtimeSegmentAssignment.assignConsumingSegment` always places a partition on one instance per replica group, whatever the slot size. This PR makes the requirement explicit in `TableConfigUtils.validateUpsertAndDedupConfig` for OFFLINE upsert tables: - An `instanceAssignmentConfigMap.OFFLINE` entry is required, it must be replica-group based, and `numInstancesPerPartition` must be `1`. - The partition column resolved for segment assignment (`TableConfigUtils.getPartitionColumn`) must be a column of the `segmentPartitionConfig`. When no partition column is configured, the `segmentPartitionConfig` must have exactly one column, matching how `SegmentUtils` resolves the partition id from segment metadata. - Pre-configured instance partitions (`instancePartitionsMap.OFFLINE`) are rejected, since their partition layout cannot be validated from the table config. This also covers the mirror server set selector, which derives the layout from the pre-configured instance partitions and ignores `numInstancesPerPartition`. `OfflineUpsertTableTest` is switched from the legacy `replicaGroupStrategyConfig` to an OFFLINE instance assignment config. The legacy config was not taking effect in that test (no replica-group `segmentAssignmentConfigMap` entry), so the table ran on balanced assignment and passed only because every segment was replicated to both servers. ## Compatibility Offline upsert tables created without an OFFLINE instance assignment config, or with a slot wider than one instance, fail validation on their next table config update. Such tables already produce incorrect upsert results whenever a partition has segments on more than one server, so the failure surfaces an existing misconfiguration rather than changing behavior of a working table. The offline upsert page in pinot-docs does not mention instance assignment yet; a docs follow-up will add the requirement with an example config. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
