twitter / fatcache

Memcache on SSD
Apache License 2.0
1.3k stars 178 forks source link

Improved command line handling of alternative maximum slab sizes. #7

Closed johnsocg closed 11 years ago

johnsocg commented 11 years ago

The first patch fixes handling of -I arguments. Traditional memcached behaviour and fatcache usage text both dictate the argument should be specified in bytes. Internally it was being treated as if it were megabytes.

The second patch, again for traditional memcached compatibilty (existing scripts, config files,etc), adds the ability to append a unit suffix k/K or m/M to specify the slab size as KiB or MiB, respectively, via the command line.

manjuraj commented 11 years ago

Thanks for the patch @johnsocg

Fatcache is designed to be different from traditional memcache. The design of the fatcache (batch writes) dictates that writes happen at slab size granularity and given SSD erase granularity, this needs to be multiple of MB. This is the reason why the slab sizes command-line argumentis is considered as multiple of MB.

johnsocg commented 11 years ago

@manjuraj

Understandable. It still doesn't change the fact that the usage dictates specifying slab size in bytes, and the value is modified internally, unbeknownst to the user, to an undocumented, arbitrary alignment.

I've added an additional commit (https://github.com/johnsocg/fatcache/commit/242ceee556b83746276874114bc0323339d59794) that only allows specifying slabs as multiples of 1MB. Otherwise, it'll exit on error the same as if a slab is too large or too small.

I believe this route is more user-friendly than just updating the usage string to say "use MB", but I don't want to spam pull requests. If you're interested in the change, I'll create a new one.

manjuraj commented 11 years ago

Ah!, I see your point. You are saying that the following usage indicates that we expect the input as bytes and not multiple of MB

-I, --slab-size=N : set slab size in bytes (default: 1048576 bytes)

We can fix the usage. However, I would like the command-line to take slab-size input as multiple of MB