gabriellefu opened a new pull request, #22982:
URL: https://github.com/apache/kafka/pull/22982

   # Changes                                                                    
                                                                                
                                                  
                                                                                
                                                                                
                                                    
     `group.consumer.assignors` accepts either built-in short names or fully 
qualified class names, but the                                                  
                                                       
     resolved assignors were never checked for name collisions. Two problems 
followed.                                                                       
                                                       
                                                                                
                                                                                
                                                    
     * Two entries could resolve to the same `name()`, including the same 
built-in configured twice: once                                                 
                                                          
       as `uniform` and once as 
`org.apache.kafka.coordinator.group.assignor.UniformAssignor`. Startup          
                                                                                
                    
       succeeded, and the failure surfaced later when `GroupMetadataManager` 
builds its name-keyed map                                                       
                                                       
       with `Collectors.toMap` and throws `IllegalStateException` for the 
duplicate key. That happens                                                     
                                                          
       while a `__consumer_offsets` partition is loading, so the coordinator 
for the partition never                                                         
                                                       
       becomes active even though the broker started cleanly.                   
                                                                                
                                                    
     * A custom assignor could return `uniform` or `range` from `name()` and 
silently take over the                                                          
                                                       
       built-in name. Members and groups reference their assignor by name, 
including the preferred                                                         
                                                         
       assignor persisted in `__consumer_offsets`, so a group that had been 
using the built-in would                                                        
                                                        
       silently switch to the custom implementation.                            
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     Client-side assignors already reject duplicate names in                    
                                                                                
                                                    
     `ConsumerPartitionAssignor#getAssignorInstances`. This adds the equivalent 
checks on the broker.           
   
   ## `GroupCoordinatorConfig#consumerGroupAssignors`                           
                                                                                
                                                  
                                                                                
                                                                                
                                                    
     * Startup fails with `ConfigException` if two entries resolve to the same 
`name()`.                                                                       
                                                     
     * Startup fails with `ConfigException` if a custom assignor reuses the 
name of a built-in assignor,                                                    
                                                        
       whether or not the built-in is itself configured.                        
                                                                                
                                                    
     * Built-ins are recognised by class rather than by how the entry was 
resolved, so configuring a                                                      
                                                          
       built-in by its fully qualified class name keeps working.                
                                                                                
                                                    
     * Both checks mirror `GroupCoordinatorConfig#streamsGroupAssignors` 
(#22920) so that the two loaders                                                
                                                           
       reject the same misconfigurations with the same messages.                
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     # Testing                                                                  
                                                                                
                                                    
                                                                                
                                                                                
                                                    
     * 
`GroupCoordinatorConfigTest#testConsumerGroupAssignorsWithDuplicateNamesFails`: 
two custom                                                                      
                                             
       assignors resolving to the same name fail; the same built-in configured 
once by name and once by                                                        
                                                     
       class name fails.                                                        
                                                                                
                                                    
     * 
`GroupCoordinatorConfigTest#testConsumerGroupAssignorsWithReservedBuiltinNameFails`:
 a custom                                                                       
                                         
       assignor returning `uniform` fails, both on its own and alongside the 
configured built-in.                                                            
                                                       
     * 
`GroupCoordinatorConfigTest#testConsumerGroupAssignorsBuiltinByClassName`: both 
built-ins                                                                       
                                             
       configured by class name still resolve.                                  
                                                                                
                                                    
     * `ConsumerIntegrationTest#testRackAwareAssignment` still passes; it 
configures a custom assignor by                                                 
                                                          
       class name on a real cluster, so it covers the startup path the new 
checks run on.               


-- 
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]

Reply via email to