技術的負債解消のため、AWS Lambdaでコードのクリーンアップに取り組んでいる話

2021年6月15日 12:00

はじめまして、エンジニアの藤田と申します。

paizaに未経験でエンジニアとして入社して約1年、まだまだなところも多いですがなんとかやっています。

最近は、JavaScript(or TypeScript)+Vue.jsを使ってフロントページを構築したり、Rails+MySQLでデータモデルを構築したり、cssと格闘したりして、いろいろなシステムレイヤーでいろいろな仕事をがんばっています。

開発上の課題について

paizaは自社開発のWebサービスなので、インクリメンタルに機能をリリースし続け、さらにその状態を維持し続けるのはとても重要なことだと考えています。ただ、開発を継続していると、どうしてもいつの間にか役割を終えてしまったコードが出てきてしまいますよね。

例えば

・新規コントローラの追加によってプロダクト上の意味がなくなった旧コントローラ・旧アクション
・旧アクションは消されたが、対応するビューファイルが消されていない
・ビューファイルは消されたが、それにのみ使用されているcss, jsファイルがそのままになっている

などは、Railsで継続的な開発を進めていると、よく見受けられる課題ではないでしょうか。

新規追加に合わせて、同時に不要となるコードが削除できればよいのですが、なかなか難しいケースもあります。もとの実装に切り戻すことになる可能性もありますし、リスク管理的にはカナリアリリースなど段階的移行をしたほうが望ましいことを考えると、機能追加と削除をアトミックに処理しようとすることにはそれほど意味はないでしょう。新規追加時は不要なものが何かわかっているので、同時に削除もしたくなりますが、そこに集中しすぎてしまうと本来の目的である新規機能開発に集中できなくなってしまいます。(このことを示す話として、アジャイル開発の文脈で名高いKent Beckは2つの帽子の例えを用いて、機能追加とリファクタリングは同時に行うべきではないことを示唆しています。)

また、そのコードがプロダクトにとって意味のないものかどうかは、その近辺のコードを担当した人であれば見分けがつきますが、そうでない人にとっては判別が難しいものです。そして担当した人がいたとしても、いずれ忘れます。これは典型的な技術的負債であり、開発のスケーラビリティやアジリティを低下させるとともに、見積もりの不確実性を高める原因となり得ます。

課題に対するアプローチ

この課題に対する対応はいくつか考えられます。

最適解としては、使われていないコードを自動的に破棄する仕掛けを作ることだと思います。RubyやPythonが実装しているガベージコレクタは、メモリというリソースに限定しますが、この課題をうまく解決しています。このガベージコレクタをソースコードというリソースに拡張すればいいわけですが、Ruby・JavaScript・Hamlなど、複数の言語を使うプロジェクトにおいて参照・非参照関係をグラフ化するのは、極めて困難です。

次の改善策としては、本番環境(プロダクションサーバ)で実行した行(カバレッジ)を蓄積して、実行されていない行をリストアップする方法が考えられます。 Rubyには、このために開発された言語機能として、OneshotCoverageがあります。本番環境で実行されなかったけど実際に生きているコード、いわば「偽陽性」のコードも混ざってしまう懸念もありますが、少なくとも判別する材料が不足しているよりはベターと言えるため、実際にやってみました。

システム名は、実行環境でのカバレッジを取得する観点からon the fly coverage(以降OTFCと略します)としました。

OTFCの目的

実行されていないRailsコードを行単位で機械的に検出し、継続的に除去できる前提を作る。

・結果、paizaを軽量に保ち開発アジリティを増加させる。
・結果、paizaを軽量に保ち開発スケーラビリティを増加させる。
・結果、paizaを軽量に保ち見積の不確実性を低減する。

OTFCの実行環境

OTFCはメインサービスではないので、OTFCによるデプロイ負荷の極端な増加や本番環境の不安定化は避けなければなりません。本番環境は最低限必要なデータを送信するだけにして、別の実行環境で処理させます。

サーバさえあればなんでもよかったわけですが、あまり管理の手間をかけたくないため、いわゆるサーバレス環境を利用しました。

・AWS lambda(デプロイ時＋定期的にデータをPOSTするときだけ処理させるのでFaaSがお得)
・AWS dynamoDB(マネージドサービスで安く使えそうなので利用する)
・AWS API Gateway (データPOSTの受け取り口、本番環境などからデータを受け取りlambdaに流す)

OTFCのテーブル設計

例えどれだけ起こりそうにないことであったとしても、不可能なことを消していき、それが最後に残ったのならそれが真実であるに違いない。　シャーロックホームズの一節より

まずは、DynamoDBのスキーマを検討しました。ホームズの引用の通り、実行された行を消していき、実行されていない行を残すことで未使用コードを浮き彫りにします。DynamoDBにはSet型というこの種のクエリに適したデータタイプがあるので、それを利用しましょう。

// これは疑似コードです。
file_path: string // 例：app/models/users.rb
file_hash: string // ソースファイル文字列のmd5値(oneshot coverageもmd5を使っているため合わせる)
line_nos_remain: Set<integer> // 実行されていない行の集合
line_nos_full: Set<integer> // ファイルの有効行の集合、カバレッジ計算などに使う予定
created_at: datetime // snapshotによりtable rowが書き換わった時刻、必須ではない
updated_at: datetime // patchによりline_nos_remainが書き換わった時刻、必須ではない
Unique index on [file_path, file_hash] // このタプルをdynamoDBのprimary keyとする

OTFCの処理の設計

処理を記述する上では、本番環境に影響を与えないことを意識します。デプロイタイミングで監視対象のファイル一式に関する情報(snapshot)をまず送出し、OneshotCoverageより定時的に送出されるカバレッジ情報(patch)をもとにテーブルを更新していきましょう。

snapshotのイメージ

{
 "command_type": "snapshot",
 "timestamp": "2021-02-01T20:15:28+09:00",
 "payload": [
   {
     "file_path": "app/models/user.rb",
     "file_hash": "2167c26f864e7dd096a325c80786346b",
     "line_nos": [2, 3, 5, 7, 8, 13, 21]
   }
 ]
}

Patchのイメージ

{
 "command_type": "patch",
 "timestamp": "2021-02-15T13:21:34+09:00",
 "payload": [
   {
     "file_path": "app/models/user.rb",
     "file_hash": "2167c26f864e7dd096a325c80786346b",
     "executed_lines": [ 2, 5, 7]
   }
 ]
}

冪等な処理を意識する

できる限り実行行に関するデータを収集するため、複数あるappサーバの全てでpatch処理を動かすことを想定します。例えば、appサーバ1とappサーバ2がカバレッジに関する似たようなpatchを送出する可能性などが十分に考えられますが、こうした重複データに対しても動くように設計する必要があります。

今回はDynamoDBが持つset型により、重複排除の処理が簡単に実現できるのでよいですが、RDBを用いる場合は検討すべきことが出てくると思います。

デプロイ耐性を持たせる

ソースコード一式の更新がなければ問題はすごく簡単で、そのままpatchを待ち受けているだけで未実行行が浮き彫りになっていきます。しかし現実には、デプロイが発生してその都度ファイルリストの更新が走ってしまうため、何も考えないと蓄積した実行行に関するデータを喪失してしまいます。

今回はLambdaでtableとsnapshotデータを比較して、単純にpathとhashが同じ場合は、table rowを更新しない方法で処理することにします。このときpathとhashは一致するけど、内容が全く異なるファイルのカバレッジ行もkeepされてしまう問題はありますが、そんなケースはめったに起こらないし、偽陰性(使われてないけど、使われているように見える)は今回のユースケースでは大した問題にはなりません。

スクリーンショット 2021-06-14 11.58.15

DBにはあるが新規snapshotにはないpathをどうするか

例えば、ソースファイルを削除するとDynamoDBにそのファイルの情報が記録されていてもカバレッジが更新されることはないので、いわば無効なデータが残留してしまいます。そのようなデータ行は明示的に削除します。

結論と今後の展望

というわけで、ひとつひとつコツコツ検討してみたことで、とりあえずベースが設計できました。アルゴリズムの難易度的には、paizaのスキルランクB〜A相当の課題だと思います。もっと緻密に設計すれば、より高水準なシステムにできるとは思いますが、今回はほどほどの設計ということでご容赦ください！

今後の展望としては

・試験運用・実運用を通じた処理の調整
・ソースコードと照らし合わせた可視化やクエリの実装
・サーバレス環境での継続的開発ができるようにする(基盤整備)

などを通じて、改善を進めていければと思います。

一緒にpaizaの未来を作ってみませんか？

paizaではエンジニア職をはじめ、複数の職種で積極採用中です。

paizaの事業と組織は、日々成長と変化を続けており、挑戦できるチャンスがたくさんあります。

もし「詳しい話を聞いてみたい」と思っていただいた方は、以下の求人リンクからご応募ください。まずはカジュアル面談で、気軽にお話しできればと思います。（現在リモートワーク推進中のため、オンラインでも実施しています）

補足：AWS Lambda(Node.js)での実装例

「正直コードがないとよくわからない」という人もいるかと思いますので、実装例も付記しておきます。

細かい話は割愛しますが、AWS DynamoDBを動かすサンプル実装として使えるかもしれません。Promise APIを用いてasync/awaitを使った実装で書いていること、BatchWriteを使ってLambdaの実行時間を削ろうとしていることが工夫点です。それ以外は、ユースケースに合わせてDynamoDBのAPIを叩いているだけです。細々した処理はテストを通すべきですが、今回は動かすことを優先して実装しました。

const DynamoDB = require('aws-sdk/clients/dynamodb');
const TABLE_NAME = 'on_the_fly_coverage';
exports.handler = async(params) => {
 const event = typeof params === 'string' ? JSON.parse(params) : params;
 // エラーが生じた場合はevent(入力)を記録し、cloudwatchで確認できるようにする
 return await run(event).catch(e => { console.log(JSON.stringify(event)); throw e; });
};
const run = async(event) => {
 const client = new DynamoDB.DocumentClient({ apiVersion: '2012-08-10', region: 'ap-northeast-1' });
 if (event.command_type === 'snapshot') { 
   return await onSnapshot(client, event);
 } else if (event.command_type === 'patch') {
   return await onPatch(client, event);
 } else {
   return { statusCode: 401, body: event, error: 'type unmatched' };
 }
};
// デプロイ時＝ソースファイルリスト一式を更新するときの処理
const onSnapshot = async(client, event) => {
 const getParams = { TableName: TABLE_NAME, AttributesToGet: ['file_path', 'file_hash'] };
 const pathAndHashInDb = (await client.scan(getParams).promise()).Items;
 const setOfPathAndHashInDb = new Set(pathAndHashInDb.map(createPathAndHash));
 const setOfPathAndHashInEvent = new Set(event.payload.map(createPathAndHash));
 
 // DBに登録されているが、新規スナップショットにない＝削除されたソースファイルなので削除する
 const setOfPathAndHashRemoved = setDifference(setOfPathAndHashInDb, setOfPathAndHashInEvent);
 const deleteItems = Array.from(setOfPathAndHashRemoved).map(record => createDeleteRequest(record[0], record[1]));
 const deleteResults = await Promise.all(arrayIntoChunks(deleteItems, 25).map(chunk => batchWriteByChunk(client, chunk)));
 // DBに登録されていないが、新規スナップショットにある＝新規作成されたファイルなので登録する
 const updatedEvent = event.payload.filter(record => !setOfPathAndHashInDb.has([record.file_path, record.file_hash]));
 const putRequests = updatedEvent.map(record => createPutRequest(client, event, record));
 const putResults = await Promise.all(arrayIntoChunks(putRequests, 25).map(chunk => batchWriteByChunk(client, chunk)));
 // DBに登録されていて、新規スナップショットにある＝変更のない既存ファイルなので、行番号情報含めkeepする
 // DBに登録されておらず、新規スナップショットにもない＝そんなファイルはないので何もしない。
 return { statusCode: 200, body: { putResults, deleteResults } };
};
const createPathAndHash = (record) => [record.file_path, record.file_hash];
// 集合の差を計算する {1,2,3,5} - {2,5} = {1,3}
const setDifference = (setA, setB) => {
 const res = new Set(setA);
 for (const elem of setB) {
   res.delete(elem);
 }
 return res;
};
const createPutRequest = (client, event, record) => ({
 PutRequest: {
   Item: {
     file_path: record.file_path,
     file_hash: record.file_hash,
     created_at: event.timestamp,
     updated_at: event.timestamp,
     line_nos_full: client.createSet(record.line_nos),
     line_nos_remain: client.createSet(record.line_nos)
   }
 }
});
const createDeleteRequest = (filePath, fileHash) => ({
 DeleteRequest: {
   Key: {
     file_path: filePath,
     file_hash: fileHash
   }
 }
});
const batchWriteByChunk = async(client, requestChunk) => {
 const params = { RequestItems: { [TABLE_NAME]: requestChunk } };
 return await client.batchWrite(params).promise();
};
// dynamoDBクエリを効率的に行うためのミニバッチ化処理
const arrayIntoChunks = (arr, chunkSize) => {
 const chunks = [];
 for (let i = 0; i < arr.length; i += chunkSize) {
   chunks.push(arr.slice(i, i + chunkSize));
 }
 return chunks;
};
// パッチ時＝OneshotCoverageのデータ受け取り時の処理
const onPatch = async(client, event) => {
 const results = await Promise.all(event.payload.map(async(record) => await updateRecord(client, event, record)));
 return { statusCode: 200, body: results };
};
// 未実行の行番号セットからカバレッジが確認された行番号を除去する
const updateRecord = async(client, event, record) => {
 const params = {
   TableName: TABLE_NAME,
   Key: { file_path: record.file_path, file_hash: record.file_hash },
   ExpressionAttributeNames: { '#updated_at': 'updated_at', '#line_nos_remain': 'line_nos_remain' },
   ExpressionAttributeValues: { 
     ':file_path': record.file_path, 
     ':file_hash': record.file_hash, 
     ':updated_at': event.timestamp, 
     ':executedLines': client.createSet(record.executed_lines), 
   },
   UpdateExpression: 'SET #updated_at = :updated_at DELETE #line_nos_remain :executedLines',
   ConditionExpression: "file_path = :file_path AND file_hash = :file_hash"
 };
 const result = await client.update(params).promise().catch(onUpdateRecordException);
 return result;
};
const onUpdateRecordException = error => {
 // 指定したfile_path, file_hashがDBにない場合エラーを無視する
 if(error.code === 'ConditionalCheckFailedException'){ 
   return {};
 }
 return error;
}

この記事が気に入ったらサポートをしてみませんか？