nck-2 / test-rep

0 stars 0 forks source link

rename force-preread daemon cli option #1288

Closed githubmanticore closed 1 year ago

githubmanticore commented 1 year ago

This issue is about command line searchd option --force-preread.

It might be better as Ilya suggested at https://gitlab.com/manticoresearch/dev/issues/753#note_174880300

--force-preread should be called something like --preread-on-startup which it actually is

and documentation should be changed to. We might not remove original option but provide alias for it.

githubmanticore commented 1 year ago

➤ Sergey Nikolaev commented:

I think it's better to move this functionality from the searchd arguments list to config as it's not something to enable sometimes. People either want to preread to have close to best performance right after starting accepting connections and then they start with --force-preread or they want faster startup and then they start w/o --force-preread. Perhaps we can make it as another argument for the access_* directives, e.g.:

access_doclists = file/mmap (default file) 
access_hitlists = file/mmap (default file) 
access_plain_attrs = mmap/mmap_preread/mmap_preread_on_start/mlock (default mmap_preread) 
access_blob_attrs = mmap/mmap_preread/mmap_preread_on_start/mlock (default mmap_preread) 

having mlock always preread on start (the assumption is that if someone explicitly says he wants it to be mlock'ed he's ready to spend some time on startup, otherwise he won't get benefits from the mlock).

What do you think? @adriannuta you might know better than others how practical it is.

Or we'd better just make it as a global searchd section directive?

githubmanticore commented 1 year ago

➤ Adrian Nuta commented:

As a global setting also is not a bad idea if it's doesn't cost us much to implement it, because one that wants to force preread and installed manticore the official way would have to edit the service file - and in most cases means the systemd generator file (file that may receive updates from us).

As a setting per index seems a complication (some indexes loaded by main thread, others by a separate thread?) and I'm not sure how many would need this granulation. We can add it later if someone (big) really needs it.

githubmanticore commented 1 year ago

➤ Sergey Nikolaev commented:

OK, let's then stick with:

searchd { 
  preread_on_startup=0/1 
} 

unless someone has any objections.