Re: [Tarantool-patches] [PATCH 1/2] Copy DSO module before load instead of symlink-ing

[Konstantin Osipov <kostja.osipov@gmail.com>]

* Kirill Yukhin <kyukhin@tarantool.org> [20/05/25 16:17]:
> Hello,
> 
> Thanks a lot fr your inputs.
> 
> My answers are inlined. Iterative patch at the bottom.
> I've force pushed updated branch.
> 
> On 23 май 21:30, Konstantin Osipov wrote:
> > * Kirill Yukhin <kyukhin@tarantool.org> [20/05/20 12:54]:
> > > +	fclose(source);
> > > +	fclose(target);
> > 
> > 
> > Please use libeio or, as last resort, sendfile().
> > 
> > Tarantool is a single threaded high-performance database and
> > application server. It's not OK to block the event loop for a few
> > hundred thousand instructions (and here we can easily get
> > millions).
> 
> As a first try, I took a look @ coio_copyfile(), but it seems
> that we cannot use anything which could yield as the routine
> might be called from within a transaction.

There is no commitment to allow plugin reload within a
transaction, and I doubt anyone would use it this way. It's a
coincidence - since Vova changed to allow everything which doesn't
yield to run within a transaction, it became allowed.

> +	off_t pos, left;
> +	for (left = st.st_size, pos = 0; left > 0;) {
> +		off_t ret = eio_sendfile_sync(dest_fd, source_fd, pos,
> +					      st.st_size);

I wonder why do you use eio_sendfile_sync()?

I don't know why coio_copyfile uses it BTW. Why is eio_sendfile()
not enough?

> +		if (ret < 0) {
> +			diag_set(SystemError, "failed to copy DSO %s to %s",
> +				 path, load_name);
> +			close(source_fd);
> +			close(dest_fd);
> +			goto error;
> +		}
> +		pos += ret;
> +		left -= ret;
> +	}
> +	close(source_fd);
> +	close(dest_fd);
>  
>  	module->handle = dlopen(load_name, RTLD_NOW | RTLD_LOCAL);
>  	if (unlink(load_name) != 0)

-- 
Konstantin Osipov, Moscow, Russia
https://scylladb.com