> When a public API interfaces has no use case where 3rd parties would > implement, such as return types or factory allocated implementations, there > should not be a compile time breaking change. In the case of IndexStatistics > there is no use case where a 3rd party could implement and provide their > instance to Geode.
In the past we have added methods to these sorts of interfaces and not considered it a breaking change. I think this came up again recently in the context of adding new methods to the GatewaySenderFactory. I think it make sense to continue that practice - in many cases there is no reasonable default method implementation. Since Geode is not calling these methods, the user will not experience any runtime errors even if they did implement these interfaces and compile against an old version of Geode. In this case, I wonder if IndexStatistics even needs long versions of those metrics? Yes, the underlying Statistics class uses long, but perhaps these stats can't ever exceed MAX_INT anyway? Anyway, I'm also ok with just adding long versions if that's what we think makes sense. -Dan ________________________________ From: Jacob Barrett <jabarr...@vmware.com> Sent: Wednesday, January 19, 2022 9:02 AM To: dev@geode.apache.org <dev@geode.apache.org> Subject: Changes to Public API Return Only Interfaces Devs, First off, this is a very forward looking discussion around something that will affect upcoming major releases when deprecating public APIs. One of the issues I am finding when investigating a few deprecated APIs for removal is that the deprecation hasn’t cascaded through to all the dependent APIs. In one specific cast, public API o.a.g.cache.query.IndexStatistics has int sized stats despite all int sized stats having been deprecated publicly for some time. Removing deprecated Statistics API results in a need to deprecate and add new methods to IndexStatistics. This is fine and easy enough to do now but it raises a question about API compatibility since adding methods to an interface is technically a compile time, or source, breaking change. Adding a method to a public API interface results in a compile failure where 3rd parties implement that interface. Java solved this by adding the default implementation concept. In this specific cast we could add default implementations to this IndexStatistics interface but I think there is an argument for not. When a public API interfaces has no use case where 3rd parties would implement, such as return types or factory allocated implementations, there should not be a compile time breaking change. In the case of IndexStatistics there is no use case where a 3rd party could implement and provide their instance to Geode. Do we want to just blanket add default method implementations to all public API interfaces changes or allow the exclusion of this changes when validating API changes? This example can see in PR 7279: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Fgeode%2Fpull%2F7279%2Ffiles%23diff-e9f3ff077afba249e36bd8eb30c3d6e2a80110c098a00e26711e2a2995751626R68&data=04%7C01%7Cdasmith%40vmware.com%7C61856af5d72048bcb0fa08d9db6d79fb%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637782085511571890%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=bIqlF5G1%2BCbv4WA5pEH91bMT8A3Z8nNiAOOggqd1DBE%3D&reserved=0 You will notice that the CI failed the API check with the following errors: Error Class org.apache.geode.cache.query.IndexStatistics: Is not source compatible Error Method org.apache.geode.cache.query.IndexStatistics.getNumberOfBucketIndexesLong(): Is not source compatible • Method added to interface Error Method org.apache.geode.cache.query.IndexStatistics.getReadLockCountLong(): Is not source compatible • Method added to interface I would love your thoughts. -Jake
