problems fixed:
* failing to read fragmentation information from jemalloc
* overflow in jemalloc fragmentation hint to the defragger
* test suite not triggering eviction after population
The original jemalloc source tree was modified to:
1. Remove the configure error that prevents nested builds.
2. Insert the Redis private Jemalloc API in order to allow the
Redis fragmentation function to work.
The problem was fixed in antirez/linenoise repository applying a patch
contributed by @lamby. Here the new version is updated in the Redis
source tree.
Close#1418Close#3322
They were under /deps since they originate from a different source tree,
however at this point they are very modified and we took ownership of
both the files making changes, fixing bugs, so there is no upgrade path
from the original code tree.
Given that, better to move the code under /src with proper dependencies
and with a more simpler editing experience.
I'm the author of this line but I can't see a good reason for it to
don't be a typo, a step of 26 should be valid with 52 bits per
coordinate, moreover the line was:
if (step > 26) step = 25;
So a step of 26 was actually already used, except when one of 27 was
computed (which is invalid) only then it was trimmed to 25 instead of
26.
All tests passing after the change.
This change is documented in deps/README.md but was lost in one way or
the other, neutralizing the benefits of 24 bytes size classes (and
others).
Close#3208.
Redis-cli handles the debugger "eval" command in a special way since
sdssplitargs() would not be ok: we need to send the Redis debugger the
whole Lua script without any parsing. However in order to later free the
argument vector inside redis-cli using just sdsfreesplitres(), we need
to allocate the array of SDS pointers using the same allocator SDS is
using, that may differ to what Redis is using.
So now a newer version of SDS exports sds_malloc() and other allocator
functions to give access, to the program it is linked to, the allocator
used internally by SDS.
This gives us a 24 bytes size class which is dict.c dictEntry size, thus
improving the memory efficiency of Redis significantly.
Moreover other non 16 bytes aligned tiny classes are added that further
reduce the fragmentation of the allocator.
Technically speaking LG_QUANTUM should be 4 on i386 / AMD64 because of
SSE types and other 16 bytes types, however we don't use those, and our
jemalloc only targets Redis.
New versions of Jemalloc will have an explicit configure switch in order
to specify the quantum value for a platform without requiring any change
to the Jemalloc source code: we'll switch to this system when available.
This change was originally proposed by Oran Agra (@oranagra) as a change
to the Jemalloc script to generate the size classes define. We ended
doing it differently by changing LG_QUANTUM since it is apparently the
supported Jemalloc method to obtain a 24 bytes size class, moreover it
also provides us other potentially useful size classes.
Related to issue #2510.
Instead of successive divisions in iteration the new code uses bitwise
magic to interleave / deinterleave two 32bit values into a 64bit one.
All tests still passing and is measurably faster, so worth it.
The GIS standard and all the major DBs implementing GIS related
functions take coordinates as x,y that is longitude,latitude.
It was a bad start for Redis to do things differently, so even if this
means that existing users of the Geo module will be required to change
their code, Redis now conforms to the standard.
Usually Redis is very backward compatible, but this is not an exception
to this rule, since this is the first Geo implementation entering the
official Redis source code. It is not wise to try to be backward
compatible with code forks... :-)
Close#2637.
The returned step was in some case not enough towards normal
coordinates (for example when our search position was was already near the
margin of the central area, and we had to match, using the east or west
neighbor, a very far point). Example:
geoadd points 67.575457940146066 -62.001317572780565 far
geoadd points 66.685439060295664 -58.925040587282297 center
georadius points 66.685439060295664 -58.925040587282297 200 km
In the above case the code failed to find a match (happens at smaller
latitudes too) even if far and center are at less than 200km.
Another fix introduced by this commit is a progressively larger area
towards the poles, since meridians are a lot less far away, so we need
to compensate for this.
The current implementation works comparably to the Tcl brute-force
stress tester implemented in the fuzzy test in the geo.tcl unit for
latitudes between -70 and 70, and is pretty accurate over +/-80 too,
with sporadic false negatives.
A more mathematically clean implementation is possible by computing the
meridian distance at the specified latitude and computing the step
according to it.
This commit simplifies the implementation in a few ways:
1. zsetScore implementation improved a bit and moved into t_zset.c where
is now also used to implement the ZSCORE command.
2. Range extraction from the sorted set remains a separated
implementation from the one in t_zset.c, but was hyper-specialized in
order to avoid accumulating results into a list and remove the ones
outside the radius.
3. A new type is introduced: geoArray, which can accumulate geoPoint
structures in a vector with power of two expansion policy. This is
useful since we have to call qsort() against it before returning the
result to the user.
4. As a result of 1, 2, 3, the two files zset.c and zset.h are now
removed, including the function to merge two lists (now handled with
functions that can add elements to existing geoArray arrays) and
the machinery used in order to pass zset results.
5. geoPoint structure simplified because of the general code structure
simplification, so we no longer need to take references to objects.
6. Not counting the JSON removal the refactoring removes 200 lines of
code for the same functionalities, with a simpler to read
implementation.
7. GEORADIUS is now 2.5 times faster testing with 10k elements and a
radius resulting in 124 elements returned. However this is mostly a
side effect of the refactoring and simplification. More speed gains
can be achieved by trying to optimize the code.
Current todo:
- replace functions in zset.{c,h} with a new unified Redis
zset access API.
Once we get the zset interface fixed, we can squash
relevant commits in this branch and have one nice commit
to merge into unstable.
This commit adds:
- Geo commands
- Tests; runnable with: ./runtest --single unit/geo
- Geo helpers in deps/geohash-int/
- src/geo.{c,h} and src/geojson.{c,h} implementing geo commands
- Updated build configurations to get everything working
- TEMPORARY: src/zset.{c,h} implementing zset score and zset
range reading without writing to client output buffers.
- Modified linkage of one t_zset.c function for use in zset.c
Conflicts:
src/Makefile
src/redis.c
This fixes issue #2535, that was actually an hiredis library bug (I
submitted an issue and fix to the redis/hiredis repo as well).
When an asynchronous hiredis connection subscribes to a Pub/Sub channel
and gets an error, and in other related conditions, the function
redisProcessCallbacks() enters a code path where the link is
disconnected, however the function returns before freeing the allocated
reply object. This causes a memory leak. The memory leak was trivial to
trigger in Redis Sentinel, which uses hiredis, every time we tried to
subscribe to an instance that required a password, in case the Sentinel
was configured either with the wrong password or without password at
all. In this case, the -AUTH error caused the leaking code path to be
executed.
It was verified with Valgrind that after this change the leak no longer
happens in Sentinel with a misconfigured authentication password.
Main reasons for upgrade:
- Remove a warning when building Redis
- Add multi pack/unpack
- Improve memory usage and use Lua allocator properly
- Fix some edge case encoding/decoding bugs
