There's a bunch of functions in view.{hh|cc} that don't belong to any
class and perform view-related claculations for view updates. Lots of
them eventually call view_info::select_statement() which will later need
the dictionary.
By now all those methods' callers have data dictionary at hand and can
share it via argument.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The goal is to have the dictionary at places that later wrap calls to
view_info::select_statement(). This graph of calls starts at the only
public view_updates::generate_update() method which, in turn, is called
from view_update_builder that already has data dictionary at hand.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The caller is table with view-update-generator at hand (it calls
mutate_MV on). Builder here is used as a temporary object that destroys
once the caller coroutine co_return-s, so keeping the database obtained
from the view-update-generator is safe.
Later the v.u.b. object will propagate its data dictionary down the
callstacks.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Caller already has it to call mutate_MV() on. The method in question
will need the generator in one of the next patches.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Both services carry database which will be used by auxiliary objects
like view_updates, view_update_builder, consumer, etc in next patches.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The AWS signature-generating code was moved from alternator some time ago as is. Now it's clear that in which places it should be extended to work for S3 client as well. The enhancements are
- Support UNSIGNED-PAYLOAD to omit calculating checksums for request body
- Include full URL path into the signature, not just hard-coded "/" string
- Don't check datastamp expiration if not asked for
This is a part of #13493Closes#13535
* github.com:scylladb/scylladb:
utils/aws: Brush up the aws_sigv4.hh header
utils/aws: Export timepoint formatter
utils/aws: Omit datestamp expiration checks when not needed
utils/aws: Add canonical-uri argument
utils/aws: Support unsigned-payload signatures
By removing unneeded headers inclusions. At the cost of few more forward
declarations and a couple of extra includes in other .cc files.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13552
Add new columns to the `system.topology` table: `shard_count` and `ignore_msb`. When a node bootstraps or restarts and observes that the values stored in `topology` are different than the local values, it updates them. This is done in the `update_topology_with_local_metadata` function (the 'metadata' here being the two values).
Additional flag persisted in `system.scylla_local` is used to safely avoid performing read barriers when the values didn't change on node restart. A comment in `update_topology_with_local_metadata` explains why this flag is needed.
An example use case where `shard_count` and `ignore_msb` are needed is creating CDC generations.
Fixes: #13508Closes#13521
* github.com:scylladb/scylladb:
raft topology: update `release_version` in topology on restart
raft topology: store `shard_count` and `ignore_msb` in topology
in this series, we use <=> operator to replace `big_decimal::compare()` for better readability. also, we trade the chained ternary expression with a more verbose if-else statement for better performance and readability.
Closes#13478
* github.com:scylladb/scylladb:
utils: big_decimal: replace compare() with <=> operator
utils: big_decimal: optimize big_decimal::compare()
Such namespace-wide imports can create conflicts between names that
are the same in seastar and std, such as {std,seastar}::future and
{std,seastar}::format, since we also have 'using namespace seastar'.
Replace the namespace imports with explicit qualification, or with
specific name imports.
Closes#13528
This is a part of PR #13493 that contains found fixes for the client code itself. The original PR has some questions to resolve, so it's worth merging the fixes separately.
Closes#13534
* github.com:scylladb/scylladb:
s3/client: Add comments about multipart upload completion message
s3/client: Fix succeeded/failed part upload final checking
s3/client: Fix parts to start from 1
Fixes https://github.com/scylladb/scylla-doc-issues/issues/823.
This PR extends the note on the Tracing page to explain what is meant by setting the RF to ALL and adds a link for reference.
Closes#12418
* github.com:scylladb/scylladb:
docs: add an explanation to recommendation in the Note box
doc: extend the information about the recommended RF on the Tracing page
Check on node start if local value of `release_version` changed. If
it did, update it in `system.topology` like we do with `shard_count` and
`ignore_msb`.
Add new columns to the `system.topology` table: `shard_count` and
`ignore_msb`. When a node bootstraps or restarts and observes that the
values stored in `topology` are different than the local values, it
updates them. This is done in the `update_topology_with_local_metadata`
function (the 'metadata' here being the two values).
Additional flag persisted in `system.scylla_local` is used to safely
avoid performing read barriers when the values didn't change on node
restart. A comment in `update_topology_with_local_metadata` explains why
this flag is needed.
An example use case where `shard_count` and `ignore_msb` are needed is
creating CDC generations.
Fixes: #13508
Add lost pragma-once directive.
Remove the hashers.hh inclusion. It was carried in when the whole code
was detached from alternator (f5de0582c8), but this header is not needed
in the header, only in the .cc file which uses sha256_hasher.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The format of timestamp for AWS requests is defined in documentation,
there's already the code that prepares it in this form. This patch
exports this method so that S3 client could use it in next patches.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The signing code is used in two ways -- by alternator to verify the
arrived signed request and by S3 client to prepare the signed request.
In the former case date expiration check is performed, but for the
latter this is not required, because date stamp is most likely now (or
close to it).
So this patch makes the orig_datestamp argument optional meaning that
expiration checks can be omited.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Current signing code hard-codes the "/" as the URL, likely this just
works for alternator. For S3 client the URL would include bucket and
object name and should thus become the argument, not constant.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
For S3 signing the whole request payload can be too resource consuming.
Fortunately, payload signing is only enforced if used with plain http,
but with real S3 we're going to use signed requests over https only (see
next patch why).
Said that, the patch turns body-content into optional reference (i.e. --
a pointer) so that the signing code could inject the UNSIGNED-PAYLOAD
mark instead of the payload signature and omit heavy payload signing.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The message length is pre-calculated in advance to provide correct
content-length request header. This math is not obvious and deserves a
comment.
Also, the final message preparation code is also implicitly checking
if any part failed to upload. There's a comment in the upload_sink's
upload_part() method about it, but the finalization place deserves one
too.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
When all parts upload complete the final message is prepared and sent
out to the server. The preparation code is also responsible for checking
if all parts uploaded OK by checking the part etag to be non-empty. In
that check a misprint crept in -- the whole list is checked to be empty,
not the individual etag itself.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently if index_node throws when trying to
add an already indexed node, pop_node might
unindex the existing node instead of the new one.
Instead, with this change, unindex_node looks up
the node by its pointer and removed it from the
index map only if it's found there so to clean up
safely after index_node throws (at any stage).
Add a unit test to verify that.
In addition, added a unit test to reproduce #13502 and test the fix.
Closes#13512
* github.com:scylladb/scylladb:
test: locator_topology: add test_update_node
topology: add_node, unindex_node: make exception safe
Docs say, that part numbers should start from 1, while the code follows
the tradition and starts from 0. Minio is conveniently incompatible in
this sense so test had been passing so far. On real S3 part number 0
ends up with failed request.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
this is a part of a series to migrating from `operator<<(ostream&, ..)` based formatting to fmtlib based formatting. the goal here is to enable fmtlib to print following classes without the help of `operator<<`.
- partition_key_view
- partition_key
- partition_key::with_schema_wrapper
- key_with_schema
- clustering_key_prefix
- clustering_key_prefix::with_schema_wrapper
the corresponding `operator<<()` are dropped dropped in this change,
as all its callers are now using fmtlib for formatting now. the helper
of `print_key()` is removed, as its only caller is
`operator<<(std::ostream&, const
clustering_key_prefix::with_schema_wrapper&)`.
the reason why all these operators are replaced in one go is that
we have a template function of `key_to_str()` in `db/large_data_handler.cc`.
this template function is actually the caller of operator<< of
`partition_key::with_schema_wrapper` and
`clustering_key_prefix::with_schema_wrapper`.
so, in order to drop either of these two operator<<, we need to remove
both of them, so that we can switch over to `fmt::to_string()` in this
template function.
Refs scylladb#13245
Closes#13513
* github.com:scylladb/scylladb:
keys: consolidate the formatter for partition_keys
keys: specialize fmt::formatter<partition_key> and friends
now that we are using C++20, it'd be more convenient if we can use
the <=> operator for comparing. the compiler creates the 6 other
operators for us if the <=> operator is defined. so the code is more
compacted.
in this change, `big_decimal::compare()` is replaced with `operator<=>`,
and its caller is updated accordingly.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
before this change in the worst case, the underlying
`number::compare()` gets called twice. as it is used by Boost::multiprecision
to implement the comparing operators of `number`. but since we can
have the result in one go, there is no need to to perform the
comparison multiple times.
so, in this change, we just call `number::compare()` explicitly,
and use it to implement `compare()`. this should save a call of
`number::compare()`. also, the chained ternary expression is
replaced using if-else statement for better readability.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The names of these states have been the source of confusion ever since they were introduced. Give them names which better reflects their true meaning and gives less room for misinterpretation. The changes are:
* active/unused -> active
* active/used -> active/need_cpu
* active/blocked -> active/await
Hopefully the new names do a better job at conveying what these states really mean:
* active - a regular admitted permit, which is active (as opposed to an inactive permit).
* active/need_cpu - an active permit which was marked as needing CPU for the read to make progress. This permit prevents admission of new permits while it is in this state.
* active/await - a former active/need_cpu permit, which has to wait on I/O or a remote shard. While in this state, it doesn't block the admission of new permits (pending other criteria such as resource availability).
Closes#13482
* github.com:scylladb/scylladb:
docs/dev/reader-concurrency-semaphore.md: expand on how the semaphore works
reader_permit: give better names to active* states
Lately we have observed that some builds are missing the package_url in the build metadata. This is usually caused by changes in how build metadata is stored on the servers and the s3 reloc server failing to dig them out of the metadata files. A user can usually still obtain the package url but currently there is no way to plug in user-obtained scylla package into the script's workflow.
This PR fixes this by allowing the user to provide the package as `$ARTIFACT_DIR/scylla.package` (in unpacked form).
Closes#13519
* github.com:scylladb/scylladb:
scripts/open-coredump.sh: allow bypassing the package downloading
scripts/open-coredump.sh: check presence of mandatory field in build json object
scripts/open-coredump.sh: more consistent error messaging
Current if index_node throws when trying to
add an already indexed node, pop_node might
unindex the existing node instead of the new one.
Instead, with this change, unindex_node looks up
the node by its pointer and removed it from the
index map only if it's found there so to clean up
safely after index_node throws (at any stage).
Add a unit test to verify that.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
scylla-sstable currently has two ways to obtain the schema:
* via a `schema.cql` file.
* load schema definition from memory (only works for system tables).
This meant that for most cases it was necessary to export the schema into a CQL format and write it to a file. This is very flexible. The sstable can be inspected anywhere, it doesn't have to be on the same host where it originates form. Yet in many cases the sstable is inspected on the same host where it originates from. In this cases, the schema is readily available in the schema tables on disk and it is plain annoying to have to export it into a file, just to quickly inspect an sstable file.
This series solves this annoyance by providing a mechanism to load schemas from the on-disk schema tables. Furthermore, an auto-detect mechanism is provided to detect the location of these schema tables based on the path of the sstable, but if that fails, the tool check the usual locations of the scylla data dir, the scylla confguration file and even looks for environment variables that tell the location of these. The old methods are still supported. In fact, if a schema.cql is present in the working directory of the tool, it is preferred over any other method, allowing for an easy force-override.
If the auto-detection magic fails, an error is printed to the console, advising the user to turn on debug level logging to see what went wrong.
A comprehensive test is added which checks all the different schema loading mechanisms. The documentation is also updated to reflect the changes.
This change breaks the backward-compatibility of the command-line API of the tool, as `--system-schema` is now just a flag, the keyspace and table names are supplied separately via the new `--keyspace` and `--table` options. I don't think this will break anybody's workflow as this tools is still lightly used, exactly because of the annoying way the schema has to be provided. Hopefully after this series, this will change.
Example:
```
$ ./build/dev/scylla sstable dump-data /var/lib/scylla/data/ks/tbl2-d55ba230b9a811ed9ae8495671e9e4f8/quarantine/me-1-big-Data.db
{"sstables":{"/var/lib/scylla/data/ks/tbl2-d55ba230b9a811ed9ae8495671e9e4f8/quarantine//me-1-big-Data.db":[{"key":{"token":"-3485513579396041028","raw":"000400000000","value":"0"},"clustering_elements":[{"type":"clustering-row","key":{"raw":"","value":""},"marker":{"timestamp":1677837047297728},"columns":{"v":{"is_live":true,"type":"regular","timestamp":1677837047297728,"value":"0"}}}]}]}}
```
As seen above, subdirectories like qurantine, staging etc are also supported.
Fixes: https://github.com/scylladb/scylladb/issues/10126Closes#13448
* github.com:scylladb/scylladb:
test/cql-pytest: test_tools.py: add tests for schema loading
test/cql-pytest: add no_autocompaction_context
docs: scylla-sstable.rst: remove accidentally added copy-pasta
docs: scylla-sstable.rst: remove paragraph with schema limitations
docs: scylla-sstable.rst: update schema section
test/cql-pytest: nodetool.py: add flush_keyspace()
tools/scylla-sstable: reform schema loading mechanism
tools/schema_loader: add load_schema_from_schema_tables()
db/schema_tables: expose types schema
Greatly expand on the details of how the semaphore works.
Organize the content into thematic chapters to improve navigation.
Improve formatting while at it.
The names of these states have been the source of confusion ever since
they were introduced. Give them names which better reflects their true
meaning and gives less room for misinterpretation. The changes are:
* active/unused -> active
* active/used -> active/need_cpu
* active/blocked -> active/await
Hopefully the new names do a better job at conveying what these states
really mean:
* active - a regular admitted permit, which is active (as opposed to
an inactive permit).
* active/need_cpu - an active permit which was marked as needing CPU for
the read to make progress. This permit prevents admission of new
permits while it is in this state.
* active/await - a former active/need_cpu permit, which has to wait on
I/O or a remote shard. While in this state, it doesn't block the
admission of new permits (pending other criteria such as resource
availability).
By allowing the user to plug a manually downloaded package. Consequently
the "package_url" field of the build metadata is checked only if there
is no user-provided extracted package.
This allows working around builds for which the metadata server returns
no "package_url", by allowing the user to locate and download the
package themselves, providing it to the script by simply extracting it
as $ARTIFACT_DIR/scylla.package.
Reproducers for https://github.com/scylladb/scylladb/issues/10770.
(Already fixed in 15ebd59071)
Includes necessary improvements and fixes to `pylib`.
Closes#12699
* github.com:scylladb/scylladb:
test/pytest: reproducers for store mutation...
test: pylib: Add a way to create cql connections with particular coordinators
test/pylib: get gossiper alive endpoints
test/topology: default replication factor 3
test/pylib: configurable replication factor
Mandatory fields missing in the build json object lead to obscure,
unrelated error messages down the road. Avoid this by checking that all
required fields all present and print an error message if any is
missing.
before this change, we just print out the addresses of the elements
in `column_defs`, if the arguments passed to `token()` function are
not valid. this is not quite helpful from the user's perspective. as
user would be more interested in the values. also, we could print
more accurate error message for different error.
in this change, following Cassandra 4.1's behavior, three cases are
identified, and corresponding errors are returned respectively:
* duplicated partition keys
* wrong order of partition key
* missing keys
where, if the partition key order is wrong, instead of printing the
keys specified by user, the correct order is printed in the error
message for helping user to correct the `token()` function.
for better performance, the checks are performed only if the keys
do not match, based on the assumption that the error handling path
is not likely to be executed.
tests are added accordingly. they tested with Canssandra 4.1.1 also.
Fixes#13468
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13470
Description of storage options is important for S3, as one
needs to know if underlying storage is either local or
remote, and if the latter, details about it.
This relies on server-side desc statement.
$ ./bin/cqlsh.py -e "describe keyspace1;"
CREATE KEYSPACE keyspace1 WITH replication = { ... } AND
storage = {'type': 'S3', 'bucket': 'sstables',
'endpoint': '127.0.0.1:9000'} AND
durable_writes = true;
Fixes#13507.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#13510
There's a legacy safety check in view code that needs to find a base table from its schema ID. To do it it calls for global storage proxy instance. The comment says that this code can be removed once computes_column feature is known by everyone. I'm not sure if that's the case, so here's more complicated yet less incompatible way to stop using global proxy instance.
Closes#13504
* github.com:scylladb/scylladb:
view: Remove unused view_ptr reference
view: Carry backing-secondary-index bit via view builder
view: Keep backing-seconday-index bool on value_getter
table: Add const index manager sgetter
in dcce0c96a9, we should have used
f-string for printing the return code of gzip subprocess. but the
"f" prefix was missed. so, in this change, it is added.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13500
