Yeah, I think you’re right, Ralph. Thanks for reasoning this over with me. Returning quickly is still a good idea.
— Matt Sicker > On Dec 29, 2022, at 01:16, Ralph Goers <ralph.go...@dslextreme.com> wrote: > > The reason a DefaultConfiguration is created and NOT the identified > configuration is that a) we would have to block while the configuration is > created and b) creating a Configuration isn’t necessarily very fast depending > on what is in it (some of which we cannot control). We do want logging to be > available quickly and only logging errors to the console during the > initialization period seems like a fair compromise. It is not at all > inconceivable that an application will have started multiple threads that > perform logging while logging is initializing and in general, we want Log4j > to be as lock free as possible. > > So your “fairly innocuous” change is not something I would support as I > explicitly rejected that idea when I implemented it in the first place. I am > also not in favor of a configuration option that makes the start up logic > even more complicated and fragile. > > Ralph > >> On Dec 28, 2022, at 6:23 PM, Matt Sicker <m...@musigma.org> wrote: >> >> And with this detail about the possibility for a Logger to be returned while >> LoggerContext::start is being executed is indeed a reason for why >> DefaultConfiguration _normally_ exists. I’m looking at two additional >> scenarios: >> >> 1. What if you provide a configuration location String or URI to the >> LoggerContext constructor? We already have a configuration source in this >> case (or at least we do if the URI corresponds to a local file), but we >> ignore this source until we start the LoggerContext. It is this situation >> where I thought it may be possible to start loading ConfigurationFactory >> plugins here to load and parse the specified configuration as the initial >> Configuration instance rather than an instance of DefaultConfiguration. This >> wasn’t previously possible due to various code dependencies, but given the >> fact that we have access to an initialized Injector instance inside the >> LoggerContext constructor (this may be a key detail I haven’t been too >> explicit about) means that we can start loading whatever subset of plugins >> we want at this point. >> >> 2. If you don’t provide a configuration location, we could still attempt to >> load the ConfigurationFactory plugins to bootstrap here. This is even more >> realistic now that we’ve removed the plugin package scanning feature, so we >> know that plugin service classes will already be available to load at this >> time. >> >> What I’m thinking here is a fairly innocuous-looking change may allow us to >> load the configuration earlier. As it is now, the LoggerContext is >> getOrCreated by the ContextSelector before Log4jContextSelector checks its >> state to see if it should be started (in which case it will then load >> ConfigurationFactory plugins to start the real configuration). What I think >> might be possible is moving this ConfigurationFactory bit deeper into the >> call chain by creating it when the LoggerContext is being created. This >> would mean the LoggerContext would have the custom configuration instance >> _before_ LoggerContext::start is invoked. >> >> One of the main tradeoffs here would be startup time I suppose. By deferring >> configuration loading until LoggerContext::start is invoked, we do indeed >> get out of the way as soon as possible to start accepting log messages and >> returning control to user code. So I suppose even if it turns out to be >> possible to load the configuration right away, this would by definition >> cause blocking as the configuration is loaded, so this would probably make >> sense as a configurable option. >> — >> Matt Sicker >> >>>> On Dec 28, 2022, at 11:44, Ralph Goers <ralph.go...@dslextreme.com> wrote: >>> >>> >>> >>>> On Dec 28, 2022, at 10:37 AM, Ralph Goers <ralph.go...@dslextreme.com> >>>> wrote: >>>> >>>> >>>> >>>> Ralph >>>> >>>>> On Dec 28, 2022, at 7:01 AM, Piotr P. Karwasz <piotr.karw...@gmail.com> >>>>> wrote: >>>>> >>>>> Hi Matt, >>>>> >>>>> On Sun, 18 Dec 2022 at 20:30, Matt Sicker <m...@musigma.org> wrote: >>>>>> During this bootstrapping, if the configuration location is available >>>>>> (such as for a unit test), should LoggerContext set up the configuration >>>>>> provided? Or is there some sort of cyclic dependency here preventing us >>>>>> from loading ConfigurationFactory right away? So far, the only cyclic >>>>>> dependencies I’ve found are related to plugins created in the >>>>>> DefaultConfiguration (or the NullConfiguration in some cases), but those >>>>>> are already commented as such (like in PatternLayout). >>>>> >>>>> I think we should rely more on our `LifeCycle` abstraction: >>>>> `Configuration` starts in the "initializing" state and does not have >>>>> any subcomponents (especially those that require a `LoggerContext` to >>>>> be present) until `initialize()` is called. >>>> >>>> Actually, a LoggerContext is hard-wired with a DefaultConfiguration. So as >>>> soon as it exists it has a Configuration. Once the Configuration >>>> identified from the configLocation is created the DefaultConfiguration is >>>> replaced. This is the discussion Matt was having in his previous email >>>> and didn’t understand why it is required to work that way. Note that once >>>> the LoggerContext is constructed it will be “wired” by the ContextSelector >>>> and will be usable by any thread. >>>> >>>>> >>>>> We can do the same thing with `LoggerContext`: after the constructor >>>>> exits it does not have a configuration and is in the "initializing" >>>>> state. >>>> >>>> After the Constructor exists is has a DefaultConfiguration. >>>> >>>>> The configuration will be created on an `initialize()` call >>>>> (maybe chained from `start()`). By then we can safely pass a reference >>>>> to `LoggerContext` to the `AbstractConfiguration` constructor. >>>>> >>>>> Logging does not occur until `start()` is called. >>>> >>>> I don’t believe this is true. Once the ContextSelector registers it any >>>> calls to log will use it. >>>> >>>> Ralph >>> >>> Note that LoggerContext.start() uses tryLock. If initialization is in >>> progress then start() will simply return. So other threads will log while >>> initialization is occurring. This is intentional. >>> >>> Ralph >> >> >
