Also, with the suggested approach, we should avoid indirectly usage of ForkJoinPool internally or set our own pool instance explicitly when using reactive things.
On Thu, Apr 8, 2021 at 1:33 PM Andrey Mashenkov <andrey.mashen...@gmail.com> wrote: > Alexey, > > I've made a PR for logger [1]. > Seems, we will need 2 logger classes. > 1. Node-aware logger adapter, that will add node prefix to messages and > delegate calls to System.Logger or whatever. > 2. Logger wrapper that will get logger from a thread-local. > > I don't like to use ThreadLocal directly when possible. > Maybe we can wrap all internal threads into IgniteThread and keep the > logger in an IgniteThread field to avoid lookups into thread-local-map. > > For user threads, only ThreadLocals can be used. > Is it really need to use thread-locals for user threads? > Will it be always obvious which node exception was thrown on? Any kind of > embedded mode? > > [1] https://github.com/apache/ignite-3/pull/87 > > On Thu, Apr 8, 2021 at 12:32 PM Alexei Scherbakov < > alexey.scherbak...@gmail.com> wrote: > >> Andrey, >> >> *final* word in the example is missing, my bad. >> >> I like the static logger approach. >> >> Regarding your comments: >> * The static logger can easily be used by multiple nodes in a single JVM, >> it's a matter of implementation. It can be achieved by setting thread >> local >> logger context for the node. >> For user threads the context can be set while entering ignite context (for >> example, by calling public API method) >> * Factory method is not necessary, because we already have a proxy object >> - >> LogWrapper, hiding internal implementation. >> >> >> >> >> >> >> >> ср, 7 апр. 2021 г. в 18:39, Andrey Mashenkov <andrey.mashen...@gmail.com >> >: >> >> > Alexei, >> > >> > I see you've merged LoggerWrapper into main and use it in Raft module. >> > I can't figure out what manner you suggest to use LoggerWrapper in. >> > In the example above 'LOG' field is static, non-final and you create a >> > wrapper explicitly. >> > >> > I see 2 ways: >> > * Use a factory method to create a logger and set it into 'static final' >> > field. >> > In this case, a user will not be able to split logs from different nodes >> > running in the same JVM. >> > * Set logger into non-static field (with dependency injection future). >> > In this case, we need to pass the logger to every class instance where >> it >> > can be used. >> > >> > >> > On Fri, Mar 26, 2021 at 6:48 PM Вячеслав Коптилин < >> > slava.kopti...@gmail.com> >> > wrote: >> > >> > > Hello Alexei, >> > > >> > > It would be nice to add something like as follows: >> > > boolean isInfoEnabled(); >> > > boolean isDebugEnabled(); >> > > or >> > > boolean isLoggable(Level) - the same way which System.Logger >> suggests >> > > >> > > Thanks, >> > > S. >> > > >> > > пт, 26 мар. 2021 г. в 17:41, Alexei Scherbakov < >> > > alexey.scherbak...@gmail.com >> > > >: >> > > >> > > > Andrey, >> > > > >> > > > I've introduced a new class LogWrapper to fix usability issues [1] >> > > > >> > > > The suggested usage is something like: >> > > > >> > > > private static LogWrapper LOG = new LogWrapper(MyClass.class); >> > > > >> > > > [1] >> > > > >> > > > >> > > >> > >> https://github.com/gridgain/apache-ignite-3/blob/9acb050a6a6a601ead849797293a1d0ad48ab9e0/modules/core/src/main/java/org/apache/ignite/lang/LogWrapper.java >> > > > >> > > > пт, 26 мар. 2021 г. в 16:05, Andrey Mashenkov < >> > > andrey.mashen...@gmail.com >> > > > >: >> > > > >> > > > > Forgot to attach a link to the PR with an example [1]. >> > > > > >> > > > > [1] https://github.com/apache/ignite-3/pull/59 >> > > > > >> > > > > On Fri, Mar 26, 2021 at 4:03 PM Andrey Mashenkov < >> > > > > andrey.mashen...@gmail.com> >> > > > > wrote: >> > > > > >> > > > > > Hi Igniters, >> > > > > > >> > > > > > In almost every new task we faced the problem of what logger >> has to >> > > be >> > > > > > used: JUL. log4J or any else. >> > > > > > >> > > > > > Since JDK 9 there is a System.Logger which interface looks >> > acceptable >> > > > for >> > > > > > use, >> > > > > > excepts maybe some usability issues like method signatures. >> > > > > > LogLevel is passed as a mandatory argument, and no shortcut >> methods >> > > are >> > > > > > provided (like 'warn', 'error' or 'info'). >> > > > > > >> > > > > > I like Alex Scherbakov idea [1] to use a brand new JDK system >> > logger >> > > by >> > > > > > default and >> > > > > > extend it with shortcut methods. >> > > > > > >> > > > > > I've created a ticket to unify logger usage in Ignite-3.0 >> project >> > to >> > > > fix >> > > > > > already existed code. >> > > > > > >> > > > > > Any thoughts or objections? >> > > > > > >> > > > > > -- >> > > > > > Best regards, >> > > > > > Andrey V. Mashenkov >> > > > > > >> > > > > >> > > > > >> > > > > -- >> > > > > Best regards, >> > > > > Andrey V. Mashenkov >> > > > > >> > > > >> > > > >> > > > -- >> > > > >> > > > Best regards, >> > > > Alexei Scherbakov >> > > > >> > > >> > >> > >> > -- >> > Best regards, >> > Andrey V. Mashenkov >> > >> >> >> -- >> >> Best regards, >> Alexei Scherbakov >> > > > -- > Best regards, > Andrey V. Mashenkov > -- Best regards, Andrey V. Mashenkov
