chenboat commented on issue #7569: URL: https://github.com/apache/pinot/issues/7569#issuecomment-951615173
Here is my followup investigation: (1) There is no unit test on the following method for a normal Pinot table let alone an upsert table. We should add unit tests to verify the behaviors of both types of tables: https://github.com/apache/pinot/blob/c9e36dd0a53370d7b8dc24459426dc77497eb8ac/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/HelixInstanceDataManager.java#L195 (2) Similarly the addSegment() method in RealtimeTableDataManager is not unit-tested in the case of upsert table segment reload: https://github.com/apache/pinot/blob/a5f3dc507e6441baca35dae5bbfad122356683b6/pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java#L358-L363 For an upsert table segment reload, today's impl first adds the segment upsert metadata (line 359-360) and then removes the upsert metadata (line 362) from the same segment copy. This sounds a redundant sequence of operations. I can not understand if the final metadata hashmap state for upsert table is correct. A unit test should verify that and lead to possible optimization here. -- 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]
