jihoonson commented on pull request #12157: URL: https://github.com/apache/druid/pull/12157#issuecomment-1014168502
@clintropolis, @cryptoe, @FrankChen021, @asdf2014 thank you for the review. The change in this PR revealed a bug in `DataNodeService` that it has duplicate "type" properties in its serialized JSON, one for the subtype key of `DruidService` and another for `serverType` in `DataNodeService`. It seems that things happened to be working before this change as the subtype key is always rewritten first in the serialized JSON and Jackson always picks up the first "type" property as the subtype key for deserialization. However, this doesn't work anymore because `DiscoveryDruidNode` now deserializes the JSON to a Map first and then converts it to a `DruidService` to ignore unknown service types. Since the map does not allow duplicate keys, the second "type" overwrites the first "type" during deserialization which caused integration test failures. This bug is a bit tricky to fix since we cannot just rename the "type" property of "DataNodeService". It will break rolling upgrade if we do that. To not break the rolling upgrade, we should ensure that the serialized JSON of `DataNodeService` has both duplicate "type" properties, but the "type" for the subtype key always appears first in the JSON. To do this, I added a custom serializer (`DruidServiceSerializer`) and a custom deserialization logic in `DiscoveryDruidNode` that uses `StringObjectPairList` as an intermediary form during deserialization. `DruidServiceSerializer` might be not required as Jackson seems to always write the subtype key first during serialization, but I added it anyway just in case where it behaves differently in some edge case. Please see the Javadoc of `DataNodeService` for more details and let me know what you think. -- 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]
