Petals Distribution

Remove dependencies to Saxon and to easycommons-utils from petals-log

Details

  • Type: Task Task
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 4.3.0-beta-1
  • Fix Version/s: 5.0.0-M1
  • Component/s: Tools
  • Security Level: Public
  • Description:
    Hide

    petals-log depends on easycommons-utils which have a static pool of Transformers used all across petals and the components.

    petals-log is present in the system classloader so the same pool is used everywhere even though components could desire to use another transformer implementation than the default one (Saxon).

    Hence, we should remove the dependency to easycommons-utils and Saxon-HE from petals-log.

    To replace the pool, we will use for now a threadlocal Transformer.
    This means one Transformer per thread to prevent concurrent access as well as permit each thread (and thus each component) to have its desired implementation.
    This means a small impact on performance during thread creation but in components threads are reused and the transformer implementation doesn't need to change so it's ok.

    Show
    petals-log depends on easycommons-utils which have a static pool of Transformers used all across petals and the components. petals-log is present in the system classloader so the same pool is used everywhere even though components could desire to use another transformer implementation than the default one (Saxon). Hence, we should remove the dependency to easycommons-utils and Saxon-HE from petals-log. To replace the pool, we will use for now a threadlocal Transformer. This means one Transformer per thread to prevent concurrent access as well as permit each thread (and thus each component) to have its desired implementation. This means a small impact on performance during thread creation but in components threads are reused and the transformer implementation doesn't need to change so it's ok.
  • Environment:
    -

Issue Links

Activity

Victor NOËL made changes - Fri, 3 Jul 2015 - 13:31:24 +0200
Field Original Value New Value
Link This issue blocks PETALSESBCONT-323 [ PETALSESBCONT-323 ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 13:32:19 +0200
Link This issue blocks PETALSESBCONT-341 [ PETALSESBCONT-341 ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 13:32:52 +0200
Status New [ 10000 ] Open [ 10002 ]
Priority Major [ 3 ]
Component/s Tools [ 10271 ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 13:32:54 +0200
Assignee Christophe DENEUX [ cdeneux ] Victor NOËL [ vnoel ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 13:32:58 +0200
Status Open [ 10002 ] In Progress [ 10003 ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 16:40:19 +0200
Status In Progress [ 10003 ] Resolved [ 10004 ]
Fix Version/s 5.0.0 [ 10412 ]
Resolution Fixed [ 1 ]
Hide
Victor NOËL added a comment - Fri, 3 Jul 2015 - 16:45:43 +0200

It's better to use a pool of transformers in petals-log

Show
Victor NOËL added a comment - Fri, 3 Jul 2015 - 16:45:43 +0200 It's better to use a pool of transformers in petals-log
Victor NOËL made changes - Fri, 3 Jul 2015 - 16:45:43 +0200
Status Resolved [ 10004 ] Open [ 10002 ]
Resolution Fixed [ 1 ]
Victor NOËL made changes - Fri, 3 Jul 2015 - 16:54:22 +0200
Status Open [ 10002 ] In Progress [ 10003 ]
Hide
Victor NOËL added a comment - Mon, 6 Jul 2015 - 16:44:38 +0200

Resolved for real: the kernel, just before setting up the logging system, will inject in PetalsPayloadDumperFileHandler (the handler that needed to access the Transformers pool before) a helper that delegate to the kernels classloader's SourceHelper and thus Transformers pool.

No more code duplication for SourceHandler and a shared Transformer pool with the kernel, which makes sense since the logging system is kind of part of the kernel.
The only reason we have to do that is that LogManager, the thing that instantiate the Handler, needs them to be instantiated in the system classloader, for security reasons mostly.

Show
Victor NOËL added a comment - Mon, 6 Jul 2015 - 16:44:38 +0200 Resolved for real: the kernel, just before setting up the logging system, will inject in PetalsPayloadDumperFileHandler (the handler that needed to access the Transformers pool before) a helper that delegate to the kernels classloader's SourceHelper and thus Transformers pool. No more code duplication for SourceHandler and a shared Transformer pool with the kernel, which makes sense since the logging system is kind of part of the kernel. The only reason we have to do that is that LogManager, the thing that instantiate the Handler, needs them to be instantiated in the system classloader, for security reasons mostly.
Victor NOËL made changes - Mon, 6 Jul 2015 - 16:44:38 +0200
Status In Progress [ 10003 ] Resolved [ 10004 ]
Resolution Fixed [ 1 ]
Hide
Victor NOËL added a comment - Tue, 7 Jul 2015 - 12:09:26 +0200 - edited

EDIT: ALL OF THIS IS ABANDONED, SEE NEXT COMMENT

So, that's not good enough: we should move petals-log to the kernel classloader and remove it from the system classloader where only petals-commons-log would stay (amongst other things).

This means providing our own subclass of LogManager responsible of:

  • Doing what org.ow2.petals.microkernel.server.PetalsServerImpl.initializeLogging() and org.ow2.petals.launcher.PetalsLauncher.setLogingSystemConfigurationFile(Configuration) are currently doing.
  • Loading the handlers from within the kernel classloader and not the system classloader as currently done by LogManager.

But also we should support adding extra handlers, such as petals-log-mongodb, with an easy procedure:

  • Adding the jar to the lib or extension or whatever directory
  • Configuring the logging.properties file with the handler
Show
Victor NOËL added a comment - Tue, 7 Jul 2015 - 12:09:26 +0200 - edited EDIT: ALL OF THIS IS ABANDONED, SEE NEXT COMMENT So, that's not good enough: we should move petals-log to the kernel classloader and remove it from the system classloader where only petals-commons-log would stay (amongst other things). This means providing our own subclass of LogManager responsible of:
  • Doing what org.ow2.petals.microkernel.server.PetalsServerImpl.initializeLogging() and org.ow2.petals.launcher.PetalsLauncher.setLogingSystemConfigurationFile(Configuration) are currently doing.
  • Loading the handlers from within the kernel classloader and not the system classloader as currently done by LogManager.
But also we should support adding extra handlers, such as petals-log-mongodb, with an easy procedure:
  • Adding the jar to the lib or extension or whatever directory
  • Configuring the logging.properties file with the handler
Victor NOËL made changes - Tue, 7 Jul 2015 - 12:09:26 +0200
Status Resolved [ 10004 ] Open [ 10002 ]
Resolution Fixed [ 1 ]
Hide
Victor NOËL added a comment - Wed, 8 Jul 2015 - 11:31:04 +0200

Ok, actually this is wrong, we go back to the first idea: having one pool for the logger handler and keeping petals-log in the system classloader: it's too much complex to change this constraint (it would mean entering in the complexity of the LogManager).

This could change in the future when switching to something like slf4j (see PETALSDISTRIB-135).

Show
Victor NOËL added a comment - Wed, 8 Jul 2015 - 11:31:04 +0200 Ok, actually this is wrong, we go back to the first idea: having one pool for the logger handler and keeping petals-log in the system classloader: it's too much complex to change this constraint (it would mean entering in the complexity of the LogManager). This could change in the future when switching to something like slf4j (see PETALSDISTRIB-135).
Victor NOËL made changes - Wed, 8 Jul 2015 - 14:01:24 +0200
Status Open [ 10002 ] In Progress [ 10003 ]
Hide
Victor NOËL added a comment - Wed, 8 Jul 2015 - 14:09:36 +0200

Commited, now we just need to add configurability through the handler's configuration.

Show
Victor NOËL added a comment - Wed, 8 Jul 2015 - 14:09:36 +0200 Commited, now we just need to add configurability through the handler's configuration.
Victor NOËL made changes - Fri, 10 Jul 2015 - 13:39:52 +0200
Status In Progress [ 10003 ] Resolved [ 10004 ]
Resolution Fixed [ 1 ]
Christophe DENEUX made changes - Tue, 6 Oct 2015 - 11:12:39 +0200
Link This issue blocks PETALSSETAL-15 [ PETALSSETAL-15 ]
Transition Status Change Time Execution Times Last Executer Last Execution Date
New New Open Open
1m 56s
1
Victor NOËL
Fri, 3 Jul 2015 - 13:32:52 +0200
Open Open In Progress In Progress
6s
1
Victor NOËL
Fri, 3 Jul 2015 - 13:32:58 +0200
In Progress In Progress Resolved Resolved
3h 7m
1
Victor NOËL
Fri, 3 Jul 2015 - 16:40:19 +0200
Resolved Resolved Open Open
5m 24s
1
Victor NOËL
Fri, 3 Jul 2015 - 16:45:43 +0200
Open Open In Progress In Progress
8m 39s
1
Victor NOËL
Fri, 3 Jul 2015 - 16:54:22 +0200
In Progress In Progress Resolved Resolved
2d 23h 50m
1
Victor NOËL
Mon, 6 Jul 2015 - 16:44:38 +0200
Resolved Resolved Open Open
19h 24m
1
Victor NOËL
Tue, 7 Jul 2015 - 12:09:26 +0200
Open Open In Progress In Progress
1d 1h 51m
1
Victor NOËL
Wed, 8 Jul 2015 - 14:01:24 +0200
In Progress In Progress Resolved Resolved
1d 23h 38m
1
Victor NOËL
Fri, 10 Jul 2015 - 13:39:52 +0200



People

Dates

  • Created:
    Fri, 3 Jul 2015 - 13:30:56 +0200
    Updated:
    Tue, 6 Oct 2015 - 11:12:39 +0200
    Resolved:
    Fri, 10 Jul 2015 - 13:39:51 +0200