We will invoke the schema builder from schema_tables.cc, and at that point, the
information about compact storage no longer exists anywhere. If we just call it
like this, it will be the same as calling it with compact_storage::no, which
will trigger a (wrong) recomputation for compact_storage::yes CFs
The best way to solve that, is make the compact_storage parameter mandatory
every time we create a new table - instead of defaulting to no. This will
ensure that the correct dense and compound calculation are always done when
calling the builder with a parameter, and not done at all when we call it
without a parameter.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
If we alter the compound property, we also have to rebuild the schema,
since some aspects of the columns depend on it. Let's just go ahead and
always rebuild the schema.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
We will use those properties during initialization - for instance, to calculate
thrift_bits.is_on_all_components. In order to do that, it has to be available at
schema creation, and not through the schema builder.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
This is basically a glorified map<sstring, sstring>, that does some validation
on the options. Analoguous structures in the past were put directly at
schema.hh, but I will keep this one separate because it got slightly more
complex than the average.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
"This still doesn't achieve compatibility with Cassandra-2.1.8, my next series
will. But let's merge this one first so the rest can be reviewed separately."
Without this, Cassandra won't even try to read our sstables. The containing
directories will be ignored.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
This table exists in 2.1.8, and although it is dropped in 2.2, we
should at least list its schema.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
2.1.8 tables have 3 more fields in their system tables, that 2.2 don't.
Since we aim at 2.1 compatibility, we have to include them.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
They do not exist in 2.2, and don't serve a huge purpose. But we will
need them for compatibility with 2.1
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
Let's leave their schema in here, since it's ready and we may need them in the
future. But since they are not present in 2.1.8, we will remove them from the
schema list.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
Don't let the current name fool you: Having this listed as "la" here
was just lack of discipline on my part. I meant by it "the format from
which we are importing" - which was named la for Origin. I wasn't
really thinking at the time that it would be dangerous to stop between
versions.
This should read ka, not la.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
Let's change the default generated tables to ka, which is the one that is present
in Origin
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
A ka file has a slightly different name on disk. Change the
parser so we can deal with both
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
When a schema is available, we use it. However, we have, by now, way too many
tests. Some of them use tables for which we don't even know the schema. It would
have been a massive amount of work to require a schema for all of them - so I am
keeping both constructors around.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
We have currently two versions of filename: one static, where the caller has to
pass all parameters, and an internal one where those parameters are derived
from the sstable attributes. Implement the latter in terms of the former so
making changes gets easier.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
It is currently only used to log a message, and for that we have an sstable
method that will do just fine. Using the name itself just makes it being passed
along throughout the captures. Remove it.
Signed-off-by: Glauber Costa <glommer@cloudius-systems.com>
Add a test case that triggers the heap overflow fixed in previous commit
("bytes_ostream: Fix current_space_left()").
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
We also allocate chunks larger than "usable_chunk_size" in alloc(). Fix
up the calculation in current_space_left().
Spotted by ASan.
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
If the requested range wraps around the end of the ring, make_local_reader()
ends up trying to create an sstable reader with that wrapping range, which
is not supported. Also our shard-looping code is wrong in a wrapping range.
The solution is simple: split the wrap-around range into two (from the start
of the range until the end of the ring, and then from the beginning of the
ring until the end of the range), and read each of these subranges normally.
This feature is needed to allow streaming a range that wraps around, because
streaming currently uses make_local_reader().
Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
make_local_reader takes a partition-range parameter, passed by *reference*.
The meaning of this passing-by-reference is not documented: when
make_local_reader returns, which of the following two statements holds?
1. make_local_reader still holds this reference, so the caller is
forced to keep the range object alive. For how long?
or
2. make_local_reader reads the range before returning, and when it
returns, the caller continues to "own" the range object, and is
allowed to do anything, including delete it.
The principle of least surprise suggests that #2 is better, unless
there is a very good reason to choose #1, and unless this requirement to
keep the argument alive (and for how long) was clearly documented.
But neither is the case here - the overhead of copying the argument is
negligable compared to the other overheads of make_local_reader, and
nothing was documented.
But it turns out the current code did #1 - the address of the range was
passed around *after* make_local_reader returns - the different readers
on different cpus continue to use it to eventually find the right sstable
byte range to iterate over. It very easy to forget this and call
make_local_reader on a on-stack range variable, and the result is a
hard-to-debug use-after-free mess.
This patch switches us to situation #2: Before make_local_reader returns,
the range is copied to whoever needs to hold it after the return, namely
each individual shard_reader. The patch to do this is trivial (one
character removed).
Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
Error reported by debug mode when running sstable test
Solution is to use unaligned cast.
dht/murmur3_partitioner.cc:67:25: runtime error: load of misaligned
address 0x6030000478fc for type 'const long int', which requires 8
byte alignment
Signed-off-by: Raphael S. Carvalho <raphaelsc@cloudius-systems.com>
It was initially created to be the function to write all sstable
components, but later on, its purpose was only to write a few
components for testing. A similar function was created in the
tests, so now it can be removed.
Signed-off-by: Raphael S. Carvalho <raphaelsc@cloudius-systems.com>
We do serialization in transport/server.cc and there's no need for
the equals() and hashCode() function so just drop ifdef'd code.
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
