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

refactor: make aio decoupled from dsn_runtime #778

Merged
merged 24 commits into from
Mar 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 11 additions & 8 deletions src/aio/disk_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,25 @@
#include <dsn/utility/flags.h>

#include "disk_engine.h"
#include "sim_aio_provider.h"
#include "runtime/service_engine.h"
#include "native_linux_aio_provider.h"

using namespace dsn::utils;

namespace dsn {
using namespace aio;

DEFINE_TASK_CODE_AIO(LPC_AIO_BATCH_WRITE, TASK_PRIORITY_COMMON, THREAD_POOL_DEFAULT)

const char *native_aio_provider = "dsn::tools::native_aio_provider";
DSN_REGISTER_COMPONENT_PROVIDER(native_linux_aio_provider, native_aio_provider);
DSN_REGISTER_COMPONENT_PROVIDER(sim_aio_provider, "dsn::tools::sim_aio_provider");

struct disk_engine_initializer
{
disk_engine_initializer() { disk_engine::instance(); }
};

// make disk_engine destructed after service_engine, which is inited in dsn_global_init,
zhangyifan27 marked this conversation as resolved.
Show resolved Hide resolved
// because service_engine relies on the former to close files.
static disk_engine_initializer disk_engine_init;

DSN_DEFINE_string("core",
aio_factory_name,
Expand Down Expand Up @@ -151,10 +157,7 @@ aio_task *disk_file::on_write_completed(aio_task *wk, void *ctx, error_code err,
disk_engine::disk_engine()
{
aio_provider *provider = utils::factory_store<aio_provider>::create(
FLAGS_aio_factory_name, dsn::PROVIDER_TYPE_MAIN, this);
if (nullptr == provider) {
dassert_f(false, "The config value of aio_factory_name is invalid");
}
native_aio_provider, dsn::PROVIDER_TYPE_MAIN, this);
Copy link
Contributor

Choose a reason for hiding this comment

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

So we don't need config aio_factory_name now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course. I will delete it in the later

_provider.reset(provider);
}

Expand Down
15 changes: 9 additions & 6 deletions src/aio/native_linux_aio_provider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@
*/

#include "native_linux_aio_provider.h"
#include "runtime/service_engine.h"

#include <dsn/tool-api/async_calls.h>
#include <dsn/c/api_utilities.h>

namespace dsn {

Expand Down Expand Up @@ -96,12 +98,17 @@ error_code native_linux_aio_provider::read(const aio_context &aio_ctx,

void native_linux_aio_provider::submit_aio_task(aio_task *aio_tsk)
{
// for the tests which use simulator need sync submit for aio
if (dsn_unlikely(service_engine::instance().is_simulator())) {
aio_internal(aio_tsk);
return;
}

tasking::enqueue(
aio_tsk->code(), aio_tsk->tracker(), [=]() { aio_internal(aio_tsk); }, aio_tsk->hash());
}

error_code native_linux_aio_provider::aio_internal(aio_task *aio_tsk,
/*out*/ uint32_t *pbytes /*= nullptr*/)
error_code native_linux_aio_provider::aio_internal(aio_task *aio_tsk)
{
aio_context *aio_ctx = aio_tsk->get_aio_context();
error_code err = ERR_UNKNOWN;
Expand All @@ -117,10 +124,6 @@ error_code native_linux_aio_provider::aio_internal(aio_task *aio_tsk,
return err;
}

if (pbytes) {
*pbytes = processed_bytes;
}

complete_io(aio_tsk, err, processed_bytes);
return err;
}
Expand Down
2 changes: 1 addition & 1 deletion src/aio/native_linux_aio_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ class native_linux_aio_provider : public aio_provider
aio_context *prepare_aio_context(aio_task *tsk) override { return new aio_context; }

protected:
error_code aio_internal(aio_task *aio, /*out*/ uint32_t *pbytes = nullptr);
error_code aio_internal(aio_task *aio);
};

} // namespace dsn
43 changes: 0 additions & 43 deletions src/aio/sim_aio_provider.cpp

This file was deleted.

45 changes: 0 additions & 45 deletions src/aio/sim_aio_provider.h

This file was deleted.

2 changes: 1 addition & 1 deletion src/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ add_library(dsn_runtime STATIC
tracer.cpp
zlocks.cpp
)
target_link_libraries(dsn_runtime PRIVATE dsn_utils sasl2 gssapi_krb5 krb5 dsn_aio)
target_link_libraries(dsn_runtime PRIVATE dsn_utils sasl2 gssapi_krb5 krb5)
install(TARGETS dsn_runtime DESTINATION "lib")
6 changes: 2 additions & 4 deletions src/runtime/service_api_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
* THE SOFTWARE.
*/

#include "aio/disk_engine.h"
#include "service_engine.h"
#include "utils/coredump.h"
#include "runtime/rpc/rpc_engine.h"
Expand Down Expand Up @@ -292,11 +291,10 @@ extern void dsn_core_init();

inline void dsn_global_init()
{
// make perf_counters/disk_engine destructed after service_engine,
// make perf_counters destructed after service_engine,
// because service_engine relies on the former to monitor
// task queues length and close files.
// task queues length.
dsn::perf_counters::instance();
dsn::disk_engine::instance();
dsn::service_engine::instance();
}

Expand Down
4 changes: 4 additions & 0 deletions src/runtime/service_engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,4 +284,8 @@ std::string service_engine::get_queue_info(const std::vector<std::string> &args)
return ss.str();
}

bool service_engine::is_simulator() const { return _simulator; }

void service_engine::set_simulator() { _simulator = true; }

} // namespace dsn
4 changes: 4 additions & 0 deletions src/runtime/service_engine.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class service_engine : public utils::singleton<service_engine>

void start_node(service_app_spec &app_spec);
const service_nodes_by_app_id &get_all_nodes() const { return _nodes_by_app_id; }
bool is_simulator() const;
void set_simulator();

private:
service_spec _spec;
Expand All @@ -131,6 +133,8 @@ class service_engine : public utils::singleton<service_engine>
dsn_handle_t _get_runtime_info_cmd;
dsn_handle_t _get_queue_info_cmd;

bool _simulator;

// <port, servicenode>
typedef std::map<int, service_node *>
node_engines_by_port; // multiple ports may share the same node
Expand Down
3 changes: 3 additions & 0 deletions src/runtime/simulator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

#include <dsn/tool/simulator.h>
#include "scheduler.h"
#include "service_engine.h"

#include "env.sim.h"
#include "runtime/task/task_engine.sim.h"
Expand Down Expand Up @@ -114,6 +115,8 @@ void simulator::install(service_spec &spec)

// the new sim_clock is taken over by unique_ptr in clock instance
utils::clock::instance()->mock(new sim_clock());

service_engine::instance().set_simulator();
}

void simulator::on_system_exit(sys_exit_type st)
Expand Down