Re: [Tarantool-patches] [PATCH v3 1/3] gc/xlog: delay xlog cleanup until relays are subscribed

[Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

24.03.2021 18:00, Cyrill Gorcunov пишет:
> On Wed, Mar 24, 2021 at 04:09:56PM +0300, Serge Petrenko wrote:
>>>    void
>>>    box_set_vinyl_memory(void)
>>>    {
>>> @@ -3000,7 +3032,7 @@ box_cfg_xc(void)
>>>    	rmean_box = rmean_new(iproto_type_strs, IPROTO_TYPE_STAT_MAX);
>>>    	rmean_error = rmean_new(rmean_error_strings, RMEAN_ERROR_LAST);
>>> -	gc_init();
>>> +	gc_init(box_check_wal_cleanup_delay());
>> You didn't  put `wal_cleanup_delay` to `dynamic_cfg_skip_at_load`,
>> and that's correct because we need to disable it if replication_anon is set.
>>
>> So wal_cleanup_delay will be reapplied once box_cfg exits.
>>
>> I propose to init gc with TIMEOUT_INFINITY then. It'd look simpler than
>> setting the same value twice IMO.
> Not sure I follow. You mean the default value in load_cfg.lua? Instead of
> 4 hours as we have now?

No, leave  the default value as is, but do not fetch it for gc_init.
Init gc with TIMEOUT_INFINITY. The default (or user-provided)
value will be applied later from `load_cfg()` anyway

>
>>> +void
>>> +gc_delay_unref(void)
>>> +{
>>> +	if (gc.is_paused) {
>>> +		assert(gc.delay_ref > 0);
>>> +		gc.delay_ref--;
>>> +		if (gc.delay_ref == 0) {
>>> +			say_info("wal/engine cleanup is resumed");
>>> +			gc.is_paused = false;
>>> +			fiber_wakeup(gc.cleanup_fiber);
>> I'd move the info message to the cleanup fiber.
>> You may deduce reason for the resume there: timeout/replicas connected and
>> print it.
>> Or don't show reason for resume at all and leave a single info message.
> OK, will do, thanks!

-- 
Serge Petrenko