[tarantool-patches] Re: [PATCH] Output of fiber.info will contain only non-idle fibers

[Георгий Кириченко]

On Thursday, July 25, 2019 12:26:40 PM MSK Konstantin Osipov wrote:
> * Георгий Кириченко <georgy at tarantool.org> [19/07/25 10:05]:
> > On Tuesday, July 23, 2019 10:56:43 PM MSK Konstantin Osipov wrote:
> > > * Maria K <marianneliash at gmail.com> [19/07/23 21:01]:
> > > > The output used to be too cluttered due to idle ones.
> > > > 
> > > > Closes #4235
> > > 
> > > @kyukhin, first, please I don't get how does this get scheduled to a
> > > milestone? How does this follow triage guidelines?
> > > 
> > > Please don't schedule anything that is not a priority, even if
> > > it's a noob issue, since it takes time of everyone involved.
> > 
> > I think anybody is free to send a patch to the public tarantool
> > mailing list despite the issue milestone (if they is not bound
> > by employee duties). Also it was a 'good first issue' ticket to
> > start a candidate on boarding.
> 
> The reason it was a good first issue is that the problem was
> wrongly defined in the first place. As defined, it had a trivial
> solution.
> 
> The mere fact we have a disagreement suggests the label was
> applied incorrectly.
> 
> > > fiber.info() already doesn't show anything from cord->dead list.
> > > Fibers which are stuck in a pool are performing application-level
> > > code, even if it's a built in pool, so contribute valuable
> > > information to fiber.info() output. Besides, it's always easy to
> > > filter out any class of fibers with luafun.
> > 
> > It is not easy to filter out such fibers. In the other hand tx fiber pool
> > is 'a hack' to spare some fiber structures between invocations. So fiber
> > pool cached fibers could/should be threatened as dead ones.
> 
> Well, I don't think everyone should be unconditionally deprived of
> this data if someone can't write a single-line snippet with luafun.
There is no point to unwind such fibers stacks which costs a lot as well as 
produces a lot of lua garbage.
> 
> > > Finally, there are other types of pools -- an application-level pool
> > > in Lua will have lots of idle fibers in it.
> > 
> > An application level fiber pool uses some user-defined condition with
> > exactly- defied meaning and state. And it isn't the same as the tx fiber
> > pool. An appplication fiber (in pool or not) is the resource managed by
> > user while tx fiber pool is not. And I see no point in seeing an idle
> > fiber from tx fiber pool.
> I disagree there is no point in seeing these fibers. They take up
> fiber stack and contribute to the total list libev events/fibers the
> scheduler has to deal with.
If you checked the code you could see that such fibers do not consume libev 
events and participated only in alive list without any scheduling duties. Such 
fibers use only fiber stack which is already accounted using memory statistics.
> 
> > > In other words, this is an partial fix of a raw feature
> > > request.
> > > 
> > > Tarantool instrumentation sucks, but it doesn't mean it should be
> > > patched by quick hacks here and there.
> > > 
> > > A nice and general solution would be to compress mostly identical
> > > fiber.info() entries. But I guess it's not a noob task.
> > 
> > I didn't find your suggestion solution nice and general in case of
> > filtering idle fibers out.
> 
> Another way of properly fixing it would be to more
> aggressively/carefully
This makes fiber pool worthless because fiber pool has only two purposes: to 
limit the amount of fibers and to spare some resources. I would like to repeat: 
a fiber cached in a fiber pool is a dead fiber without any state.
> expire such fibers from the pool.
> If you look at the current idle
> callback implementation there are a few flaws in it:
> - there is a standalone idle callback, rather than fiber idle
>   timeout, and the callback removes no more than 1 fiber per
>   second
> - when the idle callback wakes up a fiber, it doesn't necessarily
>   die. It looks at the pool->output first, and if there are
>   messages in the output, works on them. Which is apparently
>   wrong, because there always messages in the list on a busy
>   system, this doesn't mean the idle fiber should be the one to
>   work them off.
Your were right only in case when a system has a stable rps what is definitely 
not the common case. And if a system has no idle fibers (in case of stable rps) 
there is no reason to have a fiber pool.
> 
> *None* of this would be visible/bothering anybody if this
> information was hidden from fiber.info().
And we want to hide fiber pool internals from user by filtering idle (dead, 
state-less, unused, resource-less) fibers out.
> 
> So this begs the question: why am I wasting time
> discussing/explaining this, how did it suddenly become a priority,
> which I asked in the beginning of this thread.
It was only your decision to discuss this as you are free to left it out. As 
anybody else is free to submit a patch.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20190725/8e9564ff/attachment.sig>