Re: Weird test mixup
От | Michael Paquier |
---|---|
Тема | Re: Weird test mixup |
Дата | |
Msg-id | ZhNG4Io9uYOgwv3F@paquier.xyz обсуждение исходный текст |
Ответ на | Re: Weird test mixup ("Andrey M. Borodin" <x4mmm@yandex-team.ru>) |
Ответы |
Re: Weird test mixup
|
Список | pgsql-hackers |
On Sat, Apr 06, 2024 at 10:34:46AM +0500, Andrey M. Borodin wrote: > I find name of the function "injection_points_local()" strange, > because there is no verb in the name. How about > "injection_points_set_local"? That makes sense. > I'm not sure if we should refactor anything here, but > InjectionPointSharedState has singular name, plural wait_counts and > singular condition. > InjectionPointSharedState is already an array of injection points, > maybe let's add there optional pid instead of inventing separate > array of pids? Perhaps we could unify these two concepts, indeed, with a "kind" added to InjectionPointCondition. Now waits/wakeups are a different beast than the conditions that could be assigned to a point to filter if it should be executed. More runtime conditions coming immediately into my mind, that could be added to this structure relate mostly to global objects, like: - Specific database name and/or OID. - Specific role(s). So that's mostly cross-checking states coming from miscadmin.h for now. > Can we set global point to 'notice', but same local to 'wait'? Looks > like now we can't, but allowing to do so would make code simpler. You mean using the name point name with more than more callback? Not sure we'd want to be able to do that. Perhaps you're right, though, if there is a use case that justifies it. > Besides this opportunity to simplify stuff, both patches looks good > to me. Yeah, this module can be always tweaked more if necessary. Saying that, naming the new thing "condition" in InjectionPointSharedState felt strange, as you said, because it is an array of multiple conditions. For now I have applied 997db123c054 to make the GIN tests with injection points repeatable as it was an independent issue, and f587338dec87 to add the local function pieces. Attached is the last piece to switch the GIN test to use local injection points. 85f65d7a26fc should maintain the buildfarm at bay, but I'd rather not take a bet and accidently freeze the buildfarm as it would impact folks who aim at getting patches committed just before the finish line. So I am holding on this one for a few more days until we're past the freeze and the buildfarm is more stable. -- Michael
Вложения
В списке pgsql-hackers по дате отправления: