[cdc_stream] Fix issues found in tests (#40)

* [cdc_stream] Fix issues found in tests

Fixes a couple of issues found by integration testing:
- Unicode command line args in cdc_stream show up as question marks.
- Log is still named assets_stream_manager instead of cdc_stream.
- An error message contains stadia_assets_stream_manager_v3.exe.
- mount_dir was not the last arg as required by FUSE
- Promoted cache cleanup logs to INFO level since they're important
  for the proper workings of the system.
- Asset streaming cache dir is still %APPDATA%\GGP\asset_streaming.

* Address comments
This commit is contained in:
Lutz Justen
2022-12-07 11:25:43 +01:00
committed by GitHub
parent c9e18b9e91
commit c21503d21b
7 changed files with 41 additions and 17 deletions

View File

@@ -26,6 +26,7 @@
namespace cdc_ft { namespace cdc_ft {
namespace { namespace {
constexpr char kExeFilename[] = "cdc_stream.exe";
constexpr char kFuseFilename[] = "cdc_fuse_fs"; constexpr char kFuseFilename[] = "cdc_fuse_fs";
constexpr char kLibFuseFilename[] = "libfuse.so"; constexpr char kLibFuseFilename[] = "libfuse.so";
constexpr char kFuseStdoutPrefix[] = "cdc_fuse_fs_stdout"; constexpr char kFuseStdoutPrefix[] = "cdc_fuse_fs_stdout";
@@ -95,8 +96,8 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir,
if (!status.ok()) { if (!status.ok()) {
return absl::NotFoundError(absl::StrFormat( return absl::NotFoundError(absl::StrFormat(
"Required gamelet component not found. Make sure the files %s and %s " "Required gamelet component not found. Make sure the files %s and %s "
"reside in the same folder as stadia_assets_stream_manager_v3.exe.", "reside in the same folder as %s.",
kFuseFilename, kLibFuseFilename)); kFuseFilename, kLibFuseFilename, kExeFilename));
} }
std::string component_args = GameletComponent::ToCommandLineArgs(components); std::string component_args = GameletComponent::ToCommandLineArgs(components);
@@ -113,8 +114,8 @@ absl::Status CdcFuseManager::Start(const std::string& mount_dir,
RemoteUtil::QuoteForSsh(instance_), RemoteUtil::QuoteForSsh(instance_),
RemoteUtil::QuoteForSsh(component_args), remote_port, kCacheDir, RemoteUtil::QuoteForSsh(component_args), remote_port, kCacheDir,
verbosity, cleanup_timeout_sec, access_idle_timeout_sec, enable_stats, verbosity, cleanup_timeout_sec, access_idle_timeout_sec, enable_stats,
check, cache_capacity, RemoteUtil::QuoteForSsh(mount_dir), check, cache_capacity, debug ? "-d " : "", singlethreaded ? "-s " : "",
debug ? " -d" : "", singlethreaded ? " -s" : ""); RemoteUtil::QuoteForSsh(mount_dir));
bool needs_deploy = false; bool needs_deploy = false;
RETURN_IF_ERROR( RETURN_IF_ERROR(

View File

@@ -16,9 +16,30 @@
#include "cdc_stream/start_service_command.h" #include "cdc_stream/start_service_command.h"
#include "cdc_stream/stop_command.h" #include "cdc_stream/stop_command.h"
#include "cdc_stream/stop_service_command.h" #include "cdc_stream/stop_service_command.h"
#include "common/platform.h"
#include "lyra/lyra.hpp" #include "lyra/lyra.hpp"
int main(int argc, char* argv[]) { #if PLATFORM_WINDOWS
int wmain(int argc, wchar_t* wargv[]) {
// Convert args from wide to UTF8 strings.
std::vector<std::string> utf8_str_args;
utf8_str_args.reserve(argc);
for (int i = 0; i < argc; i++) {
utf8_str_args.push_back(cdc_ft::Util::WideToUtf8Str(wargv[i]));
}
// Convert args from UTF8 strings to UTF8 c-strings.
std::vector<const char*> utf8_args;
utf8_args.reserve(argc);
for (const auto& utf8_str_arg : utf8_str_args) {
utf8_args.push_back(utf8_str_arg.c_str());
}
const char** argv = utf8_args.data();
#else
int main(int argc, char** argv) {
#endif
// Set up commands. // Set up commands.
auto cli = lyra::cli(); auto cli = lyra::cli();
bool show_help = false; bool show_help = false;

View File

@@ -625,7 +625,8 @@ absl::StatusOr<std::string> MultiSession::GetCachePath(
path::Append(&appdata_path, ".cache"); path::Append(&appdata_path, ".cache");
#endif #endif
std::string base_dir = path::Join(appdata_path, "GGP", "asset_streaming"); std::string base_dir =
path::Join(appdata_path, "cdc-file-transfer", "chunks");
std::string cache_dir = GetCacheDir(src_dir); std::string cache_dir = GetCacheDir(src_dir);
size_t total_size = base_dir.size() + 1 + cache_dir.size(); size_t total_size = base_dir.size() + 1 + cache_dir.size();

View File

@@ -148,7 +148,7 @@ class MultiSession {
// |process_factory| abstracts process creation. // |process_factory| abstracts process creation.
// |data_store| can be passed for tests to override the default store used. // |data_store| can be passed for tests to override the default store used.
// By default, the class uses a DiskDataStore that writes to // By default, the class uses a DiskDataStore that writes to
// %APPDATA%\GGP\asset_streaming|<dir_derived_from_src_dir> on Windows. // %APPDATA%\cdc-file-transfer\chunks\<dir_derived_from_src_dir> on Windows.
MultiSession(std::string src_dir, SessionConfig cfg, MultiSession(std::string src_dir, SessionConfig cfg,
ProcessFactory* process_factory, ProcessFactory* process_factory,
MultiSessionMetricsRecorder const* metrics_recorder, MultiSessionMetricsRecorder const* metrics_recorder,
@@ -220,7 +220,7 @@ class MultiSession {
static std::string GetCacheDir(std::string dir); static std::string GetCacheDir(std::string dir);
// Returns the directory where manifest chunks are cached, e.g. // Returns the directory where manifest chunks are cached, e.g.
// "%APPDATA%\GGP\asset_streaming\c__path_to_game_abcdef01" for // "%APPDATA%\cdc-file-transfer\chunks\c__path_to_game_abcdef01" for
// "C:\path\to\game". // "C:\path\to\game".
// The returned path is shortened to |max_len| by removing UTF8 code points // The returned path is shortened to |max_len| by removing UTF8 code points
// from the beginning of the actual cache directory (c__path...) if necessary. // from the beginning of the actual cache directory (c__path...) if necessary.

View File

@@ -241,7 +241,7 @@ TEST_F(MultiSessionTest, GetCachePath_ContainsExpectedParts) {
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_TRUE(absl::EndsWith(*cache_path, kCacheDir)) << *cache_path; EXPECT_TRUE(absl::EndsWith(*cache_path, kCacheDir)) << *cache_path;
EXPECT_TRUE( EXPECT_TRUE(
absl::StrContains(*cache_path, path::Join("GGP", "asset_streaming"))) absl::StrContains(*cache_path, path::Join("cdc-file-transfer", "chunks")))
<< *cache_path; << *cache_path;
} }
@@ -253,7 +253,7 @@ TEST_F(MultiSessionTest, GetCachePath_ShortensLongPaths) {
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_EQ(cache_path->size(), MultiSession::kDefaultMaxCachePathLen); EXPECT_EQ(cache_path->size(), MultiSession::kDefaultMaxCachePathLen);
EXPECT_TRUE( EXPECT_TRUE(
absl::StrContains(*cache_path, path::Join("GGP", "asset_streaming"))) absl::StrContains(*cache_path, path::Join("cdc-file-transfer", "chunks")))
<< *cache_path; << *cache_path;
// The hash in the end of the path is kept and not shortened. // The hash in the end of the path is kept and not shortened.
EXPECT_EQ(cache_dir.substr(cache_dir.size() - MultiSession::kDirHashLen), EXPECT_EQ(cache_dir.substr(cache_dir.size() - MultiSession::kDirHashLen),
@@ -261,7 +261,8 @@ TEST_F(MultiSessionTest, GetCachePath_ShortensLongPaths) {
} }
TEST_F(MultiSessionTest, GetCachePath_DoesNotSplitUtfCodePoints) { TEST_F(MultiSessionTest, GetCachePath_DoesNotSplitUtfCodePoints) {
// Find out the length of the %APPDATA%\GGP\asset_streaming\" + hash part. // Find out the length of the %APPDATA%\cdc-file-transfer\chunks\" + hash
// part.
absl::StatusOr<std::string> cache_path = MultiSession::GetCachePath(""); absl::StatusOr<std::string> cache_path = MultiSession::GetCachePath("");
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
size_t base_len = cache_path->size(); size_t base_len = cache_path->size();
@@ -271,17 +272,17 @@ TEST_F(MultiSessionTest, GetCachePath_DoesNotSplitUtfCodePoints) {
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_EQ(cache_path->size(), base_len); EXPECT_EQ(cache_path->size(), base_len);
// %APPDATA%\GGP\asset_streaming\abcdefg // %APPDATA%\cdc-file-transfer\chunks\abcdefg
cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 1); cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 1);
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_EQ(cache_path->size(), base_len); EXPECT_EQ(cache_path->size(), base_len);
// %APPDATA%\GGP\asset_streaming\\u0200abcdefg // %APPDATA%\cdc-file-transfer\chunks\\u0200abcdefg
cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 2); cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 2);
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_EQ(cache_path->size(), base_len + 2); EXPECT_EQ(cache_path->size(), base_len + 2);
// %APPDATA%\GGP\asset_streaming\\u0200abcdefg // %APPDATA%\cdc-file-transfer\chunks\\u0200abcdefg
cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 3); cache_path = MultiSession::GetCachePath(u8"\u0200\u0200", base_len + 3);
ASSERT_OK(cache_path); ASSERT_OK(cache_path);
EXPECT_EQ(cache_path->size(), base_len + 2); EXPECT_EQ(cache_path->size(), base_len + 2);

View File

@@ -116,7 +116,7 @@ absl::StatusOr<std::unique_ptr<Log>> StartServiceCommand::GetLogger() {
} }
return std::make_unique<FileLog>( return std::make_unique<FileLog>(
level, GetLogPath(log_dir_.c_str(), "assets_stream_manager").c_str()); level, GetLogPath(log_dir_.c_str(), "cdc_stream").c_str());
} }
// Runs the session management service and returns when it finishes. // Runs the session management service and returns when it finishes.

View File

@@ -330,10 +330,10 @@ void DataProvider::CleanupThreadMain() {
WriterMutexLockList locks; WriterMutexLockList locks;
LockAllMutexes(&locks); LockAllMutexes(&locks);
chunks_updated_ = false; chunks_updated_ = false;
LOG_DEBUG("Starting cache cleanup"); LOG_INFO("Starting cache cleanup");
Stopwatch sw; Stopwatch sw;
absl::Status status = writer_->Cleanup(); absl::Status status = writer_->Cleanup();
LOG_DEBUG("Finished cache cleanup in %0.3f seconds", sw.ElapsedSeconds()); LOG_INFO("Finished cache cleanup in %0.3f seconds", sw.ElapsedSeconds());
next_cleanup_time = next_cleanup_time =
steady_clock_->Now() + std::chrono::seconds(cleanup_timeout_sec_); steady_clock_->Now() + std::chrono::seconds(cleanup_timeout_sec_);
absl::MutexLock cleaned_lock(&cleaned_mutex_); absl::MutexLock cleaned_lock(&cleaned_mutex_);