Re: OSGi metadata fixes for Ignite 2.x

[Łukasz Dywicki <lu...@code-house.org>]

Hello Maxim,
I missed your message, but label you made and issues referred look fine. 
I can update PR I made earlier 
(https://github.com/apache/ignite/pull/10133) which generates metadata 
and see how it goes.

In terms of best practices - I don't know any tool which can do all the 
magic automatically beside osgi tooling alone. You can for example bind 
maven coordinates to package names (groupId+artifactId=root package) 
cause that's most basic way which works without any additional plugins. 
  I am not sure if checkstyle alone can deal with that, cause it is 
working without a maven module context.
I think once we get a complete metadata we can spot it thanks to 
maven-bundle-plugin (bndtools). There is a warning emmitted when "split 
package" is found. Not sure if it will be sufficient for ignite, given 
the size of build you have.
As a reference I can point a PR I made several years ago for jackson:
https://github.com/FasterXML/oss-parent/pull/5
If you look there, the whole trick is switching maven module packaging 
to a "bundle" instead of "jar". If you do that for things which are used 
at runtime it will work.

Best,
Łukasz

On 21.07.2022 13:47, Maxim Muzafarov wrote:
> Lukasz,
> 
> I've do some investigation of related activities of the
> `split-package` issue mentioned above. All the issues related to
> moving issued packages I've marked with the 'ignite-osgi' label [1].
> Hopefully, all such packages will be moved soon.
> 
> Another question that concerns me - are there any best practices exist
> to prevent such an issue in future? Should we configure the checkstyle
> rules or something else?
> 
> [1] https://issues.apache.org/jira/issues/?jql=labels%20%3D%20ignite-osgi
> 
> On Mon, 11 Jul 2022 at 21:03, Łukasz Dywicki <lu...@code-house.org> wrote:
>>
>> Hello Maxim,
>> My replies inline
>>
>> On 11.07.2022 15:57, Maxim Muzafarov wrote:
>>> Hello Łukasz,
>>>
>>>
>>>> One of major problems which are hard to solve without cooperation is mentioned "split package" which is currently found in the sources.
>>>
>>> Can you clarify details of what is the issue of having such a case? I
>>> my understanding it will be better to consider ignite-core +
>>> ignite-indexing + ignite-calcite as a single feature. Thus having a
>>> "split package" issue should not be a problem. It will be a quite
>>> challenging to keep these modules thruly without package duplication
>>> in future.
>>
>> Split package happens when same package is defined by more than one
>> module. In such situation OSGi framework will stick with first provider
>> of a given package (ie. ignite-core), simply ignoring subsequent code
>> chunks provided by other modules (think of ignite-indexing).
>> Because OSGi validate wiring at a package level, it has some impact on
>> how code should be structured. Simply speaking module can not import a
>> foreign package and export it with additional contents. I attempted to
>> duplicate some of split package contents within ignite-indexing but it
>> should be seen as a temporary workaround.
>>
>>> I've also briefly looked at the integration with the Apache Karaf.
>>> Will it be possible to remove a module dependencies from the
>>> feature-file [4]? It seems it's a bit complicated to maintain these
>>> dependencies and corresponding versions each time some module changed.
>> These dependencies are recommended as it will force proper build order
>> of modules. Without it you may end up with a situation that feature
>> validation involving ignite-core will be made before before ignite-core
>> is being built locally. Relying on sequence order of <module> elements
>> in reactor pom is often insufficient to assure that.
>>
>>> The issue IGNITE-17046 [1] you mentioned above was merged. I've also
>>> moved the OSGi sub-modules to the Apache Ignite Extensions project
>>> [2], so I hope we can proceed here. Please, note that I've removed the
>>> ignite-paxloggin module due to it is completely out of date. The
>>> ignite-log4j will be removed very soon [3], so we have to migrate
>>> everything to the ignite-log4j2.
>> Fair enough, I can take it forward from that place and try to come with
>> a clear Karaf feature set also for some of ignite extensions. I noticed
>> some of them were refereed.
>>
>>> I've also take a look at the packages you mentioned above. Here is the
>>> list of packages with my comments:
>>>
>>> org.apache.ignite.internal.managers.systemview.walker
>>> (will be moved soon from ignite-indexing to the ignite-core)
>>>
>>> org.apache.ignite.internal.mxbean
>>> (ignite-indexing - classes deprecated and will be removed here [5])
>>>
>>> org.apache.ignite.internal.processors.cache.query
>>> (ignite-indexing - I doubt it can be simply removed currently,
>>> refactoring is required)
>>>
>>> org.apache.ignite.internal.processors.query.stat
>>> (ignite-indexing - I doubt we can do anything here)
>>>
>>> org.apache.ignite.internal.visor.cache.index
>>> (ignite-indexing - we can simply move everything to the ignite-core)
>>>
>>> org.apache.ignite.internal.visor.verify
>>> (ignite-indexing - we can simply move everything to the ignite-core)
>>>
>>> org.apache.ignite.internal.managers.systemview
>>> (will be moved soon from ignite-indexing to the ignite-core)
>>
>> I understand that moving code is difficult and causes a friction to
>> dependent projects, so easiest path is moving dependencies up (to
>> ignite-core), making it even bigger.
>> We do not need a full functinality re-factoring to satisfy "singularity"
>> of a package. It is sufficient to have unique name for package
>> structure. For example:
>> org.apache.ignite.internal.processors.cache.query in indexing could be:
>> - org.apache.ignite.internal.processors.cache.indexing.query
>> - org.apache.ignite.indexing.internal.processors.cache.query
>> or similar.
>>
>> Best,
>> Łukasz
>>
>>> [1] https://issues.apache.org/jira/browse/IGNITE-17046
>>> [2] https://issues.apache.org/jira/browse/IGNITE-13570
>>> [3] https://issues.apache.org/jira/browse/IGNITE-16650
>>> [4] https://github.com/apache/ignite-extensions/blob/master/modules/osgi-ext/osgi-karaf/src/main/resources/features.xml#L131
>>> [5] https://issues.apache.org/jira/browse/IGNITE-15761
>>>
>>> On Mon, 4 Jul 2022 at 21:42, Łukasz Dywicki <lu...@code-house.org> wrote:
>>>>
>>>> Hello Maxim,
>>>> Thank you for your answer. I haven't expected such quick feedback. I am
>>>> here to help you with OSGi, I believe that together we will be able to
>>>> get Ignite free of troubles it gives.
>>>>
>>>> I abstained from creating new issues as there are two which already
>>>> outline end user problem: IGNITE-17216, IGNITE-17216.
>>>>
>>>> I do not see a problem with moving ignite-osgi module to other
>>>> repository as long as we avoid dependency cycles.
>>>> Since igite-osgi functionality seem to be limited to bootstrapping
>>>> ignite and its classloading it does not depend per say on where code
>>>> itself is located. I suppose that osgi-karaf and osgi-paxlogging should
>>>> follow that move to keep all osgi related things in one place.
>>>>
>>>> Major concern I can raise is how to keep it up so it does not sink over
>>>> time. Integration tests I mentioned will form a safety net, but they
>>>> need to be part of build pipeline to follow changes in main repository.
>>>> Some of recent changes I found about cleaning out some inner-dependency
>>>> paths (ie. indexing-h2 relation: IGNITE-17046), are very helpful  to
>>>> solidify OSGi metadata.
>>>> One of major problems which are hard to solve without cooperation is
>>>> mentioned "split package" which is currently found in the sources. This
>>>> happens when same package is defined (and exported) from two places.
>>>> Currently both ignite-core and indexing bring contents of
>>>> org.apache.ignite.internal and spi packages:
>>>> - org.apache.ignite.internal.managers.systemview.walker
>>>> - org.apache.ignite.internal.mxbean
>>>> - org.apache.ignite.internal.processors.cache.query
>>>> - org.apache.ignite.internal.processors.query.stat
>>>> - org.apache.ignite.internal.visor.cache.index
>>>> - org.apache.ignite.internal.visor.verify
>>>> - org.apache.ignite.spi.systemview.view
>>>> To be fair, I don't know yet what is scope of these and that's where I
>>>> need most of your guidance. I don't think we will be able to stabilize
>>>> osgi integration to guarantee it will work reliably if these stay.
>>>>
>>>> Kind regards,
>>>> Łukasz
>>>>
>>>> On 4.07.2022 19:43, Maxim Muzafarov wrote:
>>>>> Hello Łukasz,
>>>>>
>>>>> Nice to meet you!
>>>>>
>>>>> Currently there are some known issues with the Apache Ignite OSGi
>>>>> integration. We have a lack of time and knowledg to support it, so any
>>>>> help are very appreciated. I also have a a plan to move this
>>>>> integration to the Apache Ignite Extensions project [1] and release it
>>>>> when all the issues are resolved. Here is an umbrella ticket for it
>>>>> [2].
>>>>>
>>>>> What do you think if I move the OSGi to the Apache Ignite Extensions
>>>>> first and after it we will resolve the issue you mentioned above? I
>>>>> can help with a review and try to do the best I can here.
>>>>> Anyway if this plan wouldn't work for you, please, feel free to create
>>>>> an issue here [3] with your PR.
>>>>>
>>>>>
>>>>> [1] https://github.com/apache/ignite-extensions
>>>>> [2] https://issues.apache.org/jira/browse/IGNITE-17019
>>>>> [3] https://issues.apache.org/jira/projects/IGNITE/
>>>>>
>>>>> On Mon, 4 Jul 2022 at 20:26, Łukasz Dywicki <lu...@code-house.org> wrote:
>>>>>>
>>>>>>
>>>>>> Hello everyone,
>>>>>> First of all thank you for awesome project and its maintenance. My name
>>>>>> is Łukasz and I am committer of Apache Karaf and Apache PLC4X projects
>>>>>> hence you most likely don't know me as I am coming from other part of
>>>>>> ASF ecosystem.
>>>>>>
>>>>>> I was asked about looking at Apache Ignite OSGi metadata as project we
>>>>>> are trying to use Ignite with is bound to the OSGi runtime. After a bit
>>>>>> of look I found that metadata generation is partial and overall
>>>>>> infrastructure in this area haven't received any polishing since a while.
>>>>>> Since there are "split packages" and few additional troubles I wanted to
>>>>>> check with you if there is a will to move that part ahead. Is there a
>>>>>> will or plan to retain the metadata or rather throw it away?
>>>>>>
>>>>>> Last week I made a draft PR with changes which allowed me to run ignite
>>>>>> inside Apache Karaf (4.3.x): https://github.com/apache/ignite/pull/10133
>>>>>> There are several maven pom updates and, so far, one code relocation in
>>>>>> kubernetes module. I based all changes on exiting properties:
>>>>>> osgi.import.package, osgi.export.package, osgi.private.package
>>>>>> I think there are few more places where "split package" is found,
>>>>>> however I did not look very carefully at these yet.
>>>>>>
>>>>>> Looking forward to hear out your guidance in how you would like to get
>>>>>> these changes, if at all. If you would be up to move that part forward I
>>>>>> can sacrifice more time in making metadata generation and validation
>>>>>> more rebust through ie. basic OSGi integration tests. PR I linked above
>>>>>> automatically validates import and export constraints for Karaf feautes.
>>>>>>
>>>>>> Kind regards,
>>>>>> Łukasz