EC-CUBE
トランザクション内で DB エラーを生じた場合にセッション変数が dtb_session に保存されない
概要(Overview)
トランザクション内で DB エラーを生じた場合にセッション変数が dtb_session に保存されない。
HTTP セッション全体を1トランザクションで処理していれば問題ないが、実際には複数のトランザクションを扱うので、DB エラーを生じたトランザクションの前にコミットされたデータと矛盾を生じ得る。(というか、実運用で生じた。dtb_shipment_item が0行の受注が、数万分の1程度の頻度で存在し、原因を分析してたどり着いた。)
dtb_session 書き込みの段階で、トランザクションを開いたままの場合には、ロールバックを行えば保存できそう (未検証)。しかし、それではトランザクション内のセッション変数の変更まで保存されてしまい、新たな矛盾を生じる。後述の再現手順の「2」まで保存される。
なお、トランザクション外でデータベースエラーを生じた場合、dtb_session に書き込まれる。これは基本的には問題ない。ただし、セッション変数との矛盾を生じうる処理においては、本件解決後は1回の変更のみでも、トランザクション内でセッション変数の変更と一緒に処理するのが (整合性の観点では) 望ましくなると思う。(上述の実運用での不具合では、この「一緒に処理する」対応も必要そう。)
期待する内容(Expect) or 要望 (Requirement)
下の再現手順で「1」は保存されるべき。 「2」は保存されなくて良い。(現状維持) 「3」は通常実行されない認識。(現状維持。エラーを無視して続行するルートがあった気もするが、通常の処理では使っていなかったと思う。)
以下の対応案を思いつく。
A案
- トランザクションを開くタイミングでセッション変数をバックアップする。
- ロールバックのタイミングでセッション変数を復元する。
- dtb_session に書き込む段階で、トランザクション内の場合、ロールバックする。
B案
- トランザクションを開くタイミングで dtb_session に書き込む。(書き込み頻度が増加する懸念)
- session_write() 関数が存在しないため、SC_Helper_Session_Ex::sfSessWrite() を自力で呼ぶ必要がありそう。
C案
- dtb_session は別のDB接続を使う。(DBセッションが2倍必要になる懸念)
- ロックにより、http://svn.ec-cube.net/open_trac/ticket/571 を回避できる可能性がある。(冒頭に書いた実運用での不具合では、この不具合も併発していた模様。)
- 『「2」は保存されなくて良い。』を満たせず、保存してしまう気がする。
再現手順(Procedure)
$objQuery =& SC_Query_Ex::getSingletonInstance();
$_SESSION['foo'] = 1; // 保存されない
$objQuery->begin();
$_SESSION['foo'] = 2; // 保存されない
$objQuery->update('x', ['x' => 'x']); // DB エラー
$_SESSION['foo'] = 3; // 保存されない
$objQuery->commit();
以下のようなエラーを生じる。
ERROR: relation "x" does not exist at character 8 STATEMENT: UPDATE x SET x= $1 ERROR: current transaction is aborted, commands ignored until end of transaction block STATEMENT: SELECT CASE WHEN EXISTS(SELECT * FROM dtb_session WHERE sess_id = $1 ) THEN 1 ELSE 0 END
環境 (environment)
- EC-CUBE: 2.13.5
- PHP: 5.4.16
- DB: PostgreSQL 9.2.24
関連情報 (Ref)
本来セッションの書き込みはアトミックであるべきだと思われますので、C案の動作が正しいと思われます。 歴史的な経緯で、2系はDBセッション採用していますが、スケールアウトしない限りはファイルセッションでも問題ないと思いますので、本不具合の解消にはファイルセッションを利用するのも良さそうです
本来セッションの書き込みはアトミックであるべきだと思われますので、C案の動作が正しいと思われます。
そんな気はします。ただ、DBコネクション2倍は、結構インパクトあるかなと。繋ぎ直すのもオーバーヘッドありそうですし、かと言って持続的接続なんかも良し悪しありそうですし。
歴史的な経緯で、2系はDBセッション採用していますが、スケールアウトしない限りはファイルセッションでも問題ないと思いますので、本不具合の解消にはファイルセッションを利用するのも良さそうです
はい。この課題の対応とは別として、実運用ではそれも有力かなと思います。 試したことありませんが、ファイルの方がパフォーマンスも良いですよね。 ただ、他の PHP アプリと session_save_path が一緒だと怖い気がするので、data/ 以下に保存するとかでしょうか。 http://svn.ec-cube.net/open_trac/ticket/621 に期待してます😁
本来セッションの書き込みはアトミックであるべきだと思われますので、C案の動作が正しいと思われます。
C案に関しましては、http://svn.ec-cube.net/open_trac/ticket/571 に有効そうなので、#498 で対応を検討したいと思います。その中で、ファイルセッションの選択肢も用意することで、DBコネクション2倍問題は許容できるのではないかと目論んでいます。 ただ、C案では、『「2」は保存されなくて良い。』を満たせない気がしてきたので、A案・B案 は引き続き対応を考えたいと思います。
メモです。
A案の以下について。
dtb_session に書き込む段階で、トランザクション内の場合、ロールバックする。
SC_Helper_Session::sfSessWrite() 内でロールバックし、連鎖して「ロールバックのタイミングでセッション変数を復元する。」が実行されても、$sess_data はセットされた後なので、バックアップしていた内容に復元されない。 現実装であれば、以下のコードで回避できる。
if ($objQuery->rollback()) {
$sess_data = session_encode();
}
しかし、#498 により、ファイルセッションに対応すると、SC_Helper_Session::sfSessWrite() を通らなくなると考えられるので、別の対応を考える必要がある。(けど、そんなのトラップできる???)
B案に関しても、ファイルセッションだと、「SC_Helper_Session_Ex::sfSessWrite() を自力で呼ぶ」は通用しなくなるので、別の対応を考える必要がありそう。
A案・B案ともボツなら、各処理側で処理順序を徹底する方向だろうか。
- トランザクション開始。
- DB処理する。
- コミット。
- セッション変数を書き換える。
いやいや、A案の「ロールバックのタイミングでセッション変数を復元する。」は、明示的なロールバックでは有効なのだから、DB エラーをトラップしたタイミングで、上述のコードでロールバックすれば、済む気がしてきた。
スケールアウトが必要な大量のトラフィックを捌く必要がある場合は、DBセッションはかえってボトルネックになりそうですので、 radis のような外部ストレージを使用するようにした方が良さそうな。
ファイルセッションに対応すると、SC_Helper_Session::sfSessWrite() を通らなくなる がよくわからないんですけど、session_set_save_handler を使えば通りますよ
@nobuhiko
ファイルセッションに対応すると、SC_Helper_Session::sfSessWrite() を通らなくなる がよくわからないんですけど、session_set_save_handler を使えば通りますよ
ファイルセッション (PHP 標準) は、session_set_save_handler() と併用可能でしたでしょうか?
@nanasess
スケールアウトが必要な大量のトラフィックを捌く必要がある場合は、DBセッションはかえってボトルネックになりそうですので、 radis のような外部ストレージを使用するようにした方が良さそうな。
Redis でしょうか。その辺りの対応は、個別サイトでのカスタマイズの範疇ですよね。それを阻害するような実装にならなければ良いのかなと思っています。 その前提で、さらに「dtb_session いらねー」という結論に合意形成できれば、dtb_session を無くす方が簡単ですね。今のところ、個人的な理想としては、そうしたいですが。
session_set_save_handlerを使わず、ファイルセッション (PHP 標準) を使うという意味なのですね。 dtb_session は常に不要なので、除く実装は何度もしていますが、session_set_save_handler を使わないとうまく動かないのでファイルセッションにした場合でもsession_set_save_handlerは使っていました。
@nobuhiko 貴重な情報ありがとうございます。 「dtb_session は常に不要なので」 なるほど(笑) そんな気はしていましたが、参考になります。 ちなみに「session_set_save_handler を使わないとうまく動かない」というのは、どういった辺りでしょうか? 個別のサイトの要件に依るカスタマイズといった部分は置いておいて、ファイルセッションを使うに当たり、多くのサイトで共通して必要だった対応などありましたら教えて下さい。(それとも、EC-CUBE が標準で session_set_save_handler() を使っているが故に、使っておかないと不都合があったなどでしょうか。)
SC_Helper_Session の __construct を消せば標準のファイルセッションになるはずですが、管理画面が動かなくなりますね
Fatal error(E_ERROR): Uncaught Error: Call to a member function doAction() on bool ってよくわからないエラーになります。
あと、以前のバージョンだと
if ($objSession->sfSessRead($sessionId) === null) {
という部分があるので、単純にやめると動かなくなりました
@nobuhiko たしか、これはDB接続が確立していないために出るエラーなので、直前で SC_Query_Ex::getSingletonIntance(); をコールすれば直ると思います
@nobuhiko
Fatal error(E_ERROR): Uncaught Error: Call to a member function doAction() on bool ってよくわからないエラーになります。
なるほど。なるほど。確かに試した際に、謎な動作だったので、サクッと削ってました。
diff --git a/data/class/helper/SC_Helper_Plugin.php b/data/class/helper/SC_Helper_Plugin.php
index bbab889..04c6395 100644
--- a/data/class/helper/SC_Helper_Plugin.php
+++ b/data/class/helper/SC_Helper_Plugin.php
@@ -91,13 +91,6 @@ class SC_Helper_Plugin
public static function getSingletonInstance($plugin_activate_flg = true)
{
if (!isset($GLOBALS['_SC_Helper_Plugin_instance'])) {
- // プラグインのローダーがDB接続を必要とするため、
- // SC_Queryインスタンス生成後のみオブジェクトを生成する。
- require_once CLASS_EX_REALDIR . 'SC_Query_Ex.php';
- if (is_null(SC_Query_Ex::getPoolInstance())) {
- return false;
- }
-
$GLOBALS['_SC_Helper_Plugin_instance'] = new SC_Helper_Plugin_Ex();
$GLOBALS['_SC_Helper_Plugin_instance']->load($plugin_activate_flg);
}
~~うろ覚えですが、自分が実装した処理で、これが無いと DB エラーとなるプラグインがあったような。もしかしたら、SQLite で動作させる的なプラグインのためだったかも・・・~~ 違いました http://svn.ec-cube.net/open_trac/ticket/1858
いずれにしても、受け取った側が false->doAction() してるのも駄目なので、手を加える必要がありそうですね。
if ($objSession->sfSessRead($sessionId) === null) {
ざっと Grep すると、 data\class\helper\SC_Helper_Mobile.php data\class\sessionfactory\SC_SessionFactory_UseRequest.php でしょうか。
実質、ガラケー用の処理な予感がしますが、「単純にやめると動かなくなりました」という状況からしますと、実行されているんですかね・・・
確かにこれで一応動きますね・・・
// ディスプレイクラス生成
$this->objDisplay = new SC_Display_Ex();
$objQuery = SC_Query_Ex::getSingletonInstance(); // これ追加
// スーパーフックポイントを実行.
$objPlugin = SC_Helper_Plugin_Ex::getSingletonInstance();
$objPlugin->doAction('LC_Page_preProcess', array($this));
プラグインは、SC_Query や SC_Query_Ex をオーバーライドできるんでしたっけ?
もしできないなら、ネックになっている部分の処理を、簡素化できそうな気がして。
そのうち調べようかと思いますが、ご存知でしたら教えて下さい。