[Bucardo-general] Questions on `add sync` params

[David E. Wheeler]

On Oct 18, 2012, at 7:46 AM, Greg Sabino Mullane <greg at endpoint.com> wrote:

> On Wed, Oct 17, 2012 at 08:50:30PM -0700, David E. Wheeler wrote:
> 
> [limitdbs]
>>> Verdict: placeholder.
>> 
>> So, it does not actually do anything now? Should I create an issue for it?
> 
> No need. It only becomes useful when/if we go to multiple kids. So I 
> suppose since we hav a good upgrade system we could simply take it 
> out for now.

Done in f8dfebe56eb970f7a1bab0d7fe83a85a1b44b76e. I also added support for dropping columns to upgrade(). Was dead simple and worked for me in a quick test of a database I built a day or two ago.

>> You've noticed all the issues I've created, I assume. :-)
> 
> Yep. Might want put this somewhere in the docs:
> 
>>  http://github.com/bucardo/bucardo/issues

Done in a7d841fede6f3fb144c669cb3aa9427967868b56.

> 
> [do_listen]
>> Did it used to do something different? Should I make it an alias for ping?
> 
> I've no idea. I say keep 'ping' and remove do_listen, with no alias. Or 
> nuke ping and keep do_listen, if you think it's a better name. I don't think 
> either one is hard-coded into people's brains enough to require an alias.

Well, I documented the `ping` parameter as:

> Boolean indicating whether or not to ping each database before each sync. Optional.

Is that correct? I will select one or the other depending on your answer. :-)

>> Well, since 5.0 is so close to done, perhaps we shouldn't worry about 
>> getting rid of the source vs. target split just now, yes? In which case, 
>> does the makedelta stuff need to be hooked back up?
> 
> Yes, it should. But source and target really no longer applies in the 
> world of b5. Probably the easiest thing to do at the moment is to make 
> a single sync-level boolean for makedelta, and expand it in a future 
> version. The expansion will allow setting things at the db+sync level, 
> which means a new internal map table to tie the two together, plus 
> a lot of interface/doc fiddling.

Okay, so, to summarize, I think this means that I should:

* Remove the makedelta param to `add db`
* Replace `source_makedelta` and `target_makedelta` with just `makedelta` in `add sync`

Yes. And how should I document it? Currently I have:

> Turns makedelta magic on or off. Value must be one of "on" or "off". Defaults to "on".

But that means nothing to me. I do not understand what makedelta does, or what it's for.

>>>> The `overdue` and `expired` params appear to be no-ops. True?
>>> 
>>> No - they are used by bucardo_report and especially for Nagios tie-ins.
>> 
>> Okay? What for? What do they mean?
> 
> They are the amount of time before a warning and critical are given. 
> (Or however your monitoring system chooses to interpret it). In other 
> words, if a sync has not run in 6 minutes, and overdue is set to 
> '5 minutes', the number of stale syncs is incremented by one and 
> that sync is reported in the special Nagios section of bucardo-report. 
> Check out that code for more details.

I can't make sense of it. But for overdue, I'm making it:

> An interval specifying the amount of time after which the sync has not run that it should be considered overdue.

And expired is what? Amount of time that a sync has been running before it's considered to have run for too long? Or is it the same as overdue, but it's CRITICAL while overdue is WARNING?

>>> It's tracking how long a sync run takes. I'm not sure how useful it 
>>> still is. If it's a lot of trouble to get working right in bucardo5, 
>>> I say we take a quick poll to see if anyone is using it and nuke it.
>> 
>> Is Bucardo currently logging that information somewhere?
> 
> Summary sync information is logged to the syncrun table, but if track_rates 
> is on, much more detail is logged, via the bucardo_rate table. Again, 
> I am fine with taking this out for now.

If it is tracking, and it might be useful, why not leave it in? Does this information currently show up in the report?

>> Yeah, but the code for customselect says to use customcols instead. customcols 
>> seems to implicitly turn on customselect, no?
> 
> customselect just needs to go away completely. See line 5793 of Bucardo.pm, 
> for example. :)

Oh, so it's a no-op. I will rip it out.

David