Hi Satish, Thanks for the review! Yes, we won't be enabling the deprecated config as dynamic. Removed the remote.log.manager.thread.pool.size config from the KIP.
Currently, the config is not marked as deprecated which was already proposed in KIP-950, we can mark that config as deprecated in the source code. Thanks, Kamal On Thu, Nov 7, 2024 at 9:23 PM Satish Duggana <satish.dugg...@gmail.com> wrote: > Thanks Kamal for the KIP. This is useful for dynamically changing the > thread pool configurations, especially in production environments. We > can skip remote.log.manager.thread.pool.size as it is already > deprecated, and remote.log.manager.copier.thread.pool.size, > remote.log.manager.expiration.thread.pool.size are derived from that. > > ~Satish. > > > On Thu, 7 Nov 2024 at 10:07, Kamal Chandraprakash > <kamal.chandraprak...@gmail.com> wrote: > > > > Hi all, > > > > If there are no more comments, then I'll start a voting thread as the > > change is minor. > > > > -- > > Kamal > > > > On Thu, Nov 7, 2024 at 9:00 AM Kamal Chandraprakash < > > kamal.chandraprak...@gmail.com> wrote: > > > > > Hi Federico, > > > > > > Updated the KIP by replacing the `isInitialized` to `isReady` in the > KIP. > > > > > > > > > > > > On Wed, Nov 6, 2024 at 12:47 PM Federico Valeri <fedeval...@gmail.com> > > > wrote: > > > > > >> Thanks Kamal, LGTM, but you should replace all instances of > > >> isInitialized to isReady in the rest of the KIP. > > >> > > >> On Wed, Nov 6, 2024 at 5:22 AM Kamal Chandraprakash > > >> <kamal.chandraprak...@gmail.com> wrote: > > >> > > > >> > Hi Federico, > > >> > > > >> > Thanks for the review! > > >> > > > >> > 1. Changed the API name to `isReady` > > >> > 2. Added an example of stacktrace in the KIP. > > >> > > > >> > PTAL. > > >> > > > >> > Thanks, > > >> > Kamal > > >> > > > >> > On Mon, Nov 4, 2024 at 2:37 PM Federico Valeri < > fedeval...@gmail.com> > > >> wrote: > > >> > > > >> > > Hi Kamal, these changes make sense to me. Thanks. > > >> > > > > >> > > In this case, I wonder if "isReady" could be a better name, > instead of > > >> > > "isInitialized". Wdyt? > > >> > > > > >> > > Could you please add an example of the stack trace that the RLMM > can > > >> > > raise during the initialization phase? > > >> > > > > >> > > On Sun, Nov 3, 2024 at 4:50 PM Kamal Chandraprakash > > >> > > <kamal.chandraprak...@gmail.com> wrote: > > >> > > > > > >> > > > Hi all, > > >> > > > > > >> > > > I would like to start a discussion thread on KIP-1105 > > >> > > > < > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1105%3A+Make+remote+log+manager+thread-pool+configs+dynamic > > >> > > >. > > >> > > > This KIP is about > > >> > > > > > >> > > > 1. Configuring the thread-pool used by the remote-log manager > > >> dynamically > > >> > > > and > > >> > > > 2. Graceful handling of remote-log components during server > startup. > > >> > > > > > >> > > > > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1105%3A+Make+remote+log+manager+thread-pool+configs+dynamic > > >> > > > > > >> > > > Please take a look and suggest your thoughts. > > >> > > > > > >> > > > Thanks, > > >> > > > Kamal > > >> > > > > >> > > > >
