Hi Rui,
> I see java8 doesn't have `Arrays.equals(int[] a, int aFromIndex, int
> aToIndex, int[] b, int bFromIndex, int bToIndex)`,
> and java11 has it. Do you have any other suggestions for java8?
Maybe use `ByteBuffer.wrap`?
ByteBuffer.wrap(array, ..., ...).equals(ByteBuffer.wrap(array2, ..., ...))
This shouldn't have overheads as far as I remember.
Or implement your own loop? It shouldn't be more than a couple of lines.
Best,
Piotrek
czw., 9 lis 2023 o 06:43 Rui Fan <1996fan...@gmail.com> napisał(a):
> Hi Piotr, Archit, Feng and Hangxiang:
>
> Thanks a lot for your feedback!
>
> Following is my comment, please correct me if I misunderstood anything!
>
> To Piotr:
>
> > Is there a reason why you are suggesting to copy out bytes from `buf` to
> `bytes`,
> > instead of using `Arrays.equals(int[] a, int aFromIndex, int aToIndex,
> int[] b, int bFromIndex, int bToIndex)`?
>
> I see java8 doesn't have `Arrays.equals(int[] a, int aFromIndex, int
> aToIndex, int[] b, int bFromIndex, int bToIndex)`,
> and java11 has it. Do you have any other suggestions for java8?
>
> Also, this code doesn't run in production. As the comment of
> System.lineSeparator():
>
> > On UNIX systems, it returns {@code "\n"}; on Microsoft
> > Windows systems it returns {@code "\r\n"}.
>
> So Mac and Linux just return one character, we will compare
> one byte directly.
>
>
>
> To Feng:
>
> > Will they be written to the taskManager.log file by default
> > or the taskManager.out file?
>
> I prefer LOG as the default value for taskmanager.system-out.mode.
> It's useful for job stability and doesn't introduce significant impact to
> users. Also, our production has already used this feature for
> more than 1 years, it works well.
>
> However, I write the DEFAULT as the default value for
> taskmanager.system-out.mode, because when the community introduces
> new options, the default value often selects the original behavior.
>
> Looking forward to hear more thoughts from community about this
> default value.
>
> > If we can make taskmanager.out splittable and rolling, would it be
> > easier for users to use this feature?
>
> Making taskmanager.out splittable and rolling is a good choice!
> I have some concerns about it:
>
> 1. Users may also want to use LOG.info in their code and just
> accidentally use System.out.println. It is possible that they will
> also find the logs directly in taskmanager.log.
> 2. I'm not sure whether the rolling strategy is easy to implement.
> If we do it, it's necessary to define a series of flink options similar
> to log options, such as: fileMax(how many files should be retained),
> fileSize(The max size each file), fileNamePatten (The suffix of file
> name),
> 3. Check the file size periodically: all logs are written by log plugin,
> they can check the log file size after writing. However, System.out
> are written directly. And flink must start a thread to check the latest
> taskmanager.out size periodically. If it's too quick, most of job aren't
> necessary. If it's too slow, the file size cannot be controlled properly.
>
> Redirect it to LOG.info may be a reasonable and easy choice.
> The user didn't really want to log into taskmanager.out, it just
> happened by accident.
>
>
>
> To Hangxiang:
>
> > 1. I have a similar concern as Feng. Will we redirect to another log file
> > not taskManager.log ?
>
> Please see my last comment, thanks!
>
> > taskManager.log contains lots of important information like init log. It
> > will be rolled quickly if we redirect out and error here.
>
> IIUC, this issue isn't caused by System.out, and it can happen if user
> call a lot of LOG.info. As I mentioned before: the user didn't really want
> to log into taskmanager.out, it just happened by accident.
> So, if users change the System.out to LOG.info, it still happen.
>
> > 2. Since we have redirected to LOG mode, Could we also log the subtask
> info
> > ? It may help us to debug granularly.
>
> I'm not sure what `log the subtask info` means. Let me confirm with you
> first.
> Do you mean like this: LOG.info("taskName {} : {}", taskName,
> userLogContext)?
>
> Best,
> Rui
>
> On Thu, Nov 9, 2023 at 11:47 AM Hangxiang Yu <master...@gmail.com> wrote:
>
> > Hi, Rui.
> > Thanks for the proposal. It sounds reasonable.
> > I have some questions, PTAL:
> > 1. I have a similar concern as Feng. Will we redirect to another log file
> > not taskManager.log ?
> > taskManager.log contains lots of important information like init log. It
> > will be rolled quickly if we redirect out and error here.
> > 2. Since we have redirected to LOG mode, Could we also log the subtask
> info
> > ? It may help us to debug granularly.
> >
> > On Thu, Nov 9, 2023 at 9:47 AM Feng Jin <jinfeng1...@gmail.com> wrote:
> >
> > > Hi, Rui.
> > >
> > > Thank you for initiating this proposal.
> > >
> > > I have a question regarding redirecting stdout and stderr to LOG:
> > >
> > > Will they be written to the taskManager.log file by default or the
> > > taskManager.out file?
> > > If we can make taskmanager.out splittable and rolling, would it be
> easier
> > > for users to use this feature?
> > >
> > > Best,
> > > Feng
> > >
> > > On Thu, Nov 9, 2023 at 3:15 AM Archit Goyal
> <argo...@linkedin.com.invalid
> > >
> > > wrote:
> > >
> > > > Hi Rui,
> > > >
> > > > Thanks for the proposal.
> > > >
> > > > The proposed solution of supporting System out and err to be
> redirected
> > > to
> > > > LOG or discarded and introducing an enum and two options to manage
> > this,
> > > > seems reasonable.
> > > >
> > > > +1
> > > >
> > > > Thanks,
> > > > Archit Goyal
> > > >
> > > >
> > > > From: Piotr Nowojski <pnowoj...@apache.org>
> > > > Date: Wednesday, November 8, 2023 at 7:38 AM
> > > > To: dev@flink.apache.org <dev@flink.apache.org>
> > > > Subject: Re: [DISCUSS] FLIP-390: Support System out and err to be
> > > > redirected to LOG or discarded
> > > > Hi Rui,
> > > >
> > > > Thanks for the proposal.
> > > >
> > > > +1 I don't have any major comments :)
> > > >
> > > > One nit. In `SystemOutRedirectToLog` in this code:
> > > >
> > > > System.arraycopy(buf, count - LINE_SEPARATOR_LENGTH,
> bytes,
> > 0,
> > > > LINE_SEPARATOR_LENGTH);
> > > > return Arrays.equals(LINE_SEPARATOR_BYTES, bytes)
> > > >
> > > > Is there a reason why you are suggesting to copy out bytes from `buf`
> > to
> > > > `bytes`,
> > > > instead of using `Arrays.equals(int[] a, int aFromIndex, int
> aToIndex,
> > > > int[] b, int bFromIndex, int bToIndex)`?
> > > >
> > > > Best,
> > > > Piotrek
> > > >
> > > > śr., 8 lis 2023 o 11:53 Rui Fan <1996fan...@gmail.com> napisał(a):
> > > >
> > > > > Hi all!
> > > > >
> > > > > I would like to start a discussion of FLIP-390: Support System out
> > and
> > > > err
> > > > > to be redirected to LOG or discarded[1].
> > > > >
> > > > > In various production environments, either cloud native or physical
> > > > > machines, the disk space that Flink TaskManager can use is limited.
> > > > >
> > > > > In general, the flink users shouldn't use the `System.out.println`
> in
> > > > > production,
> > > > > however this may happen when the number of Flink jobs and job
> > > developers
> > > > > is very large. Flink job may use System.out to output a large
> amount
> > of
> > > > > data
> > > > > to the taskmanager.out file. This file will not roll, it will
> always
> > > > > increment.
> > > > > Eventually the upper limit of what the TM can be used for is
> reached.
> > > > >
> > > > > We can support System out and err to be redirected to LOG or
> > discarded,
> > > > > the LOG can roll and won't increment forever.
> > > > >
> > > > > This feature is useful for SREs who maintain Flink environments,
> they
> > > can
> > > > > redirect System.out to LOG by default. Although the cause of this
> > > problem
> > > > > is
> > > > > that the user's code is not standardized, for SRE, pushing users to
> > > > modify
> > > > > the code one by one is usually a very time-consuming operation.
> It's
> > > also
> > > > > useful for job stability where System.out is accidentally used.
> > > > >
> > > > > Looking forward to your feedback, thanks~
> > > > >
> > > > > [1]
> > > >
> > >
> >
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fx%2F4guZE&data=05%7C01%7Cargoyal%40linkedin.com%7C937821de7bd846e6b97408dbe070beae%7C72f988bf86f141af91ab2d7cd011db47%7C0%7C0%7C638350547072823674%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=zEv6B0Xiq2SNuU6Fm%2BAXnH%2BRILbm6Q0uDRbN7h6iaPM%3D&reserved=0
> > > > <https://cwiki.apache.org/confluence/x/4guZE>
> > > > >
> > > > > Best,
> > > > > Rui
> > > > >
> > > >
> > >
> >
> >
> > --
> > Best,
> > Hangxiang.
> >
>
