Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

get_producers unittest #9923

Merged
merged 8 commits into from
Jan 20, 2021
Merged

get_producers unittest #9923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7b19c37
Moving eosio_system_tester to the testing library so it can be used b…
brianjohnson5972 Jan 12, 2021
b0889bc
Bug fix for lookup of secondary key to primary key.
brianjohnson5972 Jan 12, 2021
e1d09e5
Added test for chain_plugin get_producers
brianjohnson5972 Jan 14, 2021
54779bd
update get_producers test
huangminghuang Jan 16, 2021
a00f2d3
Added method to construct a full primary key from a prefix and a uint…
brianjohnson5972 Jan 19, 2021
de92ab0
Added method to retrieve a code/scope/table/primary_key from whatever…
brianjohnson5972 Jan 19, 2021
0765d2d
Added another test to verify get_producers logic for retrieving from …
brianjohnson5972 Jan 19, 2021
6d12177
Fixed logic for constructing the full primary key
brianjohnson5972 Jan 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions libraries/chain/backing_store/db_key_value_format.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,31 @@ namespace eosio { namespace chain { namespace backing_store { namespace db_key_v
bytes composite_key = kt ? create_prefix_type_key(scope, table, *kt) : create_prefix_key(scope, table);
return create_full_key(composite_key, code);
}

eosio::session::shared_bytes create_full_primary_key(const eosio::session::shared_bytes& prefix, uint64_t primary_key) {
const auto prefix_size = detail::prefix_size<eosio::session::shared_bytes>();
b1::chain_kv::bytes composite_key;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you need dynamic memory allocation here. I would use something like the following to eliminate the dynamic memory and branches.

char composite_key[sizeof(prmary_key) + type_size] = { static_cast<char>(key_type::primary) } ;
int prerix_contains_key_type =  (prefix.size() == prefix_size + type_size);
EOS_ASSERT(prefix.size() == prefix_size || ( prerix_contains_key_type &&   static_cast<key_type>(prefix[prefix.size() - 1]) == key_type::primary_key), ....);
memcpy(composite_key + type_size, &primary_key, sizeof(prmary_key)); 
return eosio::session::make_shared_bytes<std::string_view, 2>(
        {  std::string_view{prefix.data(),  prefix.size()},
            std::string_view{composite_key + prerix_contains_key_type, 
                                         sizeof(composite_key) - prerix_contains_key_type} });

Copy link
Contributor

@brianjohnson5972 brianjohnson5972 Jan 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for needing the dynamic memory is that initially all this code was written around chain_kv code. Since the new rocksdb session code was added afterward, the decision was to take that existing code and massage it to the new format (with the intrinsic type char and contract name at the beginning). This code was designed to prevent constructing the whole key from scratch, since they would all have the same prefix. All this code definitely needs to be refactored and cleaned up, and there are a lot of copied code, but at least it has the same pattern of code. I think it would be better to remove this method and just construct the key from scratch rather than have a highly optimized method that is different than other functions performing similar actions, and this is only being used for chain_plugin RPC calls, so not an highly optimized path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, code without branches is easier to read. I am OK if you insisted on the existing code.

constexpr static auto type_size = sizeof(key_type);
static_assert( type_size == 1, "" ); // this changing will break check below of prefix.back()
if (prefix.size() == prefix_size + type_size) {
const key_type kt = static_cast<key_type>(prefix[prefix.size() - 1]);
EOS_ASSERT(kt == key_type::primary, bad_composite_key_exception,
"DB intrinsic create_full_primary_key should only be called with a prefix for a primary key, this has a prefix for type: ${type}",
("type", static_cast<uint64_t>(kt)));
composite_key.reserve(detail::key_size(key_type::primary));
}
else {
EOS_ASSERT(prefix.size() == prefix_size, bad_composite_key_exception,
"DB intrinsic create_full_primary_key should only be called with a prefix of code, scope, and table and optionally a primary type indicator");
composite_key.reserve(type_size + detail::key_size(key_type::primary));
composite_key.push_back(static_cast<char>(key_type::primary));
}

b1::chain_kv::append_key(composite_key, primary_key);
return eosio::session::make_shared_bytes<std::string_view, 2>({std::string_view{prefix.data(),
prefix.size()},
std::string_view{composite_key.data(),
composite_key.size()}});

}
}}}} // namespace eosio::chain::backing_store::db_key_value_format
1 change: 1 addition & 0 deletions libraries/chain/include/eosio/chain/backing_store/db_key_value_format.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,7 @@ namespace chain { namespace backing_store { namespace db_key_value_format {

eosio::session::shared_bytes create_full_primary_key(name code, name scope, name table, uint64_t primary_key);
eosio::session::shared_bytes create_full_prefix_key(name code, name scope, name table, std::optional<key_type> kt = std::optional<key_type>{});
eosio::session::shared_bytes create_full_primary_key(const eosio::session::shared_bytes& prefix, uint64_t primary_key);

template<typename Key>
eosio::session::shared_bytes create_full_secondary_key(name code, name scope, name table, const Key& sec_key, uint64_t primary_key) {
Expand Down
32 changes: 32 additions & 0 deletions libraries/chain/include/eosio/chain/combined_database.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#include <eosio/chain/account_object.hpp>
#include <eosio/chain/backing_store.hpp>
#include <eosio/chain/backing_store/chain_kv_payer.hpp>
#include <eosio/chain/backing_store/db_key_value_format.hpp>
#include <eosio/chain/block_summary_object.hpp>
#include <eosio/chain/code_object.hpp>
#include <eosio/chain/contract_table_objects.hpp>
Expand Down Expand Up @@ -134,6 +136,36 @@ namespace eosio { namespace chain {
auto &get_kv_undo_stack(void) const { return kv_undo_stack; }
backing_store_type get_backing_store() const { return backing_store; }

template<typename Lambda>
bool get_primary_key_data(name code, name scope, name table, uint64_t primary_key, Lambda&& process_data) const {
if (backing_store == backing_store_type::CHAINBASE) {
const auto* t_id = db.find<chain::table_id_object, chain::by_code_scope_table>( boost::make_tuple( code, scope, table ) );
if ( !t_id ) {
return false;
}

const auto* obj = db.find<chain::key_value_object, chain::by_scope_primary>(boost::make_tuple(t_id->id, primary_key) );

if (obj) {
return process_data(obj->payer, obj->value.data(), obj->value.size());
}
}
else {
using namespace eosio::chain;
EOS_ASSERT(backing_store == backing_store_type::ROCKSDB,
chain::contract_table_query_exception,
"Support for configured backing_store has not been added");
auto full_primary_key = chain::backing_store::db_key_value_format::create_full_primary_key(code, scope, table, primary_key);
auto session = get_kv_undo_stack()->top();
auto value = session.read(full_primary_key);
if (value) {
backing_store::payer_payload pp{value->data(), value->size()};
return process_data(pp.payer, pp.value, pp.value_size);
}
}
return false;
}

private:
void add_contract_tables_to_snapshot(const snapshot_writer_ptr& snapshot) const;
void read_contract_tables_from_snapshot(const snapshot_reader_ptr& snapshot);
Expand Down
Loading