From 9d29714514da25b3c94fd7c0acb804920500680c Mon Sep 17 00:00:00 2001 From: anand Date: Mon, 8 Jun 2015 00:19:00 +0530 Subject: [PATCH 214/234] glusterd: Get the local txn_info based on trans_id in op_sm call backs. Issue: when two or more transactions are running concurrently in op_sm, global op_info might get corrupted. Fix: Get local txn_info based on trans_id instead of using global txn_info for commands (replace_brick, re-balance, profile ) which are using op_sm in originator. TODO: Handle errors properly in call backs and completely remove the global op_info from op_sm. Backportof: >Change-Id: I9d61388acc125841ddc77e2bd560cb7f17ae0a5a >BUG: 1229139 >Signed-off-by: anand Change-Id: I34dd174b5c71779664d71a0f15995a205271c42f BUG: 1230525 Signed-off-by: anand Reviewed-on: https://code.engineering.redhat.com/gerrit/52564 Reviewed-by: Atin Mukherjee Tested-by: Atin Mukherjee --- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 15 ++++++- xlators/mgmt/glusterd/src/glusterd-rpc-ops.c | 55 ++++++++++++-------------- xlators/mgmt/glusterd/src/glusterd-syncop.c | 17 -------- xlators/mgmt/glusterd/src/glusterd-utils.c | 32 +++++++-------- xlators/mgmt/glusterd/src/glusterd-utils.h | 4 -- 5 files changed, 53 insertions(+), 70 deletions(-) diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 4619f57..b1ee5e3 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -3996,6 +3996,7 @@ glusterd_op_ac_send_stage_op (glusterd_op_sm_event_t *event, void *ctx) char *op_errstr = NULL; glusterd_op_t op = GD_OP_NONE; uint32_t pending_count = 0; + glusterd_op_info_t txn_op_info = {{0},}; this = THIS; GF_ASSERT (this); @@ -4047,8 +4048,18 @@ glusterd_op_ac_send_stage_op (glusterd_op_sm_event_t *event, void *ctx) goto out; } - if (op == GD_OP_REPLACE_BRICK) - glusterd_rb_use_rsp_dict (NULL, rsp_dict); + if (op == GD_OP_REPLACE_BRICK) { + ret = glusterd_get_txn_opinfo (&event->txn_id, &txn_op_info); + if (ret) { + gf_msg (this->name, GF_LOG_ERROR, 0, + GD_MSG_TRANS_OPINFO_GET_FAIL, + "Failed to get txn_op_info " + "for txn_id = %s", uuid_utoa (event->txn_id)); + goto out; + } + + glusterd_rb_use_rsp_dict (txn_op_info.op_ctx, rsp_dict); + } rcu_read_lock (); cds_list_for_each_entry_rcu (peerinfo, &priv->peers, uuid_list) { diff --git a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c index 0a144b9..50820d7 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c @@ -1131,6 +1131,7 @@ __glusterd_stage_op_cbk (struct rpc_req *req, struct iovec *iov, xlator_t *this = NULL; glusterd_conf_t *priv = NULL; uuid_t *txn_id = NULL; + glusterd_op_info_t txn_op_info = {{0},}; this = THIS; GF_ASSERT (this); @@ -1200,6 +1201,14 @@ out: gf_msg_debug (this->name, 0, "transaction ID = %s", uuid_utoa (*txn_id)); + ret = glusterd_get_txn_opinfo (txn_id, &txn_op_info); + if (ret) { + gf_msg (this->name, GF_LOG_ERROR, 0, + GD_MSG_TRANS_OPINFO_GET_FAIL, + "Failed to get txn_op_info " + "for txn_id = %s", uuid_utoa (*txn_id)); + } + rcu_read_lock (); peerinfo = glusterd_peerinfo_find (rsp.uuid, NULL); if (peerinfo == NULL) { @@ -1233,7 +1242,7 @@ out: switch (rsp.op) { case GD_OP_REPLACE_BRICK: - glusterd_rb_use_rsp_dict (NULL, dict); + glusterd_rb_use_rsp_dict (txn_op_info.op_ctx, dict); break; } @@ -1268,7 +1277,7 @@ int32_t __glusterd_commit_op_cbk (struct rpc_req *req, struct iovec *iov, int count, void *myframe) { - gd1_mgmt_commit_op_rsp rsp = {{0},}; + gd1_mgmt_commit_op_rsp rsp = {{0},}; int ret = -1; int32_t op_ret = -1; glusterd_op_sm_event_type_t event_type = GD_OP_EVENT_NONE; @@ -1279,6 +1288,7 @@ __glusterd_commit_op_cbk (struct rpc_req *req, struct iovec *iov, xlator_t *this = NULL; glusterd_conf_t *priv = NULL; uuid_t *txn_id = NULL; + glusterd_op_info_t txn_op_info = {{0},}; this = THIS; GF_ASSERT (this); @@ -1348,6 +1358,14 @@ __glusterd_commit_op_cbk (struct rpc_req *req, struct iovec *iov, gf_msg_debug (this->name, 0, "transaction ID = %s", uuid_utoa (*txn_id)); + ret = glusterd_get_txn_opinfo (txn_id, &txn_op_info); + if (ret) { + gf_msg (this->name, GF_LOG_ERROR, 0, + GD_MSG_TRANS_OPINFO_GET_FAIL, + "Failed to get txn_op_info " + "for txn_id = %s", uuid_utoa (*txn_id)); + } + rcu_read_lock (); peerinfo = glusterd_peerinfo_find (rsp.uuid, NULL); if (peerinfo == NULL) { @@ -1377,49 +1395,26 @@ __glusterd_commit_op_cbk (struct rpc_req *req, struct iovec *iov, } } else { event_type = GD_OP_EVENT_RCVD_ACC; + GF_ASSERT (rsp.op == txn_op_info.op); + switch (rsp.op) { case GD_OP_REPLACE_BRICK: - ret = glusterd_rb_use_rsp_dict (NULL, dict); - if (ret) - goto unlock; - break; - - case GD_OP_SYNC_VOLUME: - ret = glusterd_sync_use_rsp_dict (NULL, dict); + ret = glusterd_rb_use_rsp_dict (txn_op_info.op_ctx, dict); if (ret) goto unlock; break; case GD_OP_PROFILE_VOLUME: - ret = glusterd_profile_volume_use_rsp_dict (NULL, dict); - if (ret) - goto unlock; - break; - - case GD_OP_GSYNC_SET: - ret = glusterd_gsync_use_rsp_dict (NULL, dict, rsp.op_errstr); - if (ret) - goto unlock; - break; - - case GD_OP_STATUS_VOLUME: - ret = glusterd_volume_status_copy_to_op_ctx_dict (NULL, dict); + ret = glusterd_profile_volume_use_rsp_dict (txn_op_info.op_ctx, dict); if (ret) goto unlock; break; case GD_OP_REBALANCE: case GD_OP_DEFRAG_BRICK_VOLUME: - ret = glusterd_volume_rebalance_use_rsp_dict (NULL, dict); - if (ret) - goto unlock; - break; - - case GD_OP_HEAL_VOLUME: - ret = glusterd_volume_heal_use_rsp_dict (NULL, dict); + ret = glusterd_volume_rebalance_use_rsp_dict (txn_op_info.op_ctx, dict); if (ret) goto unlock; - break; default: diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c index a54073e..1d5deb8 100644 --- a/xlators/mgmt/glusterd/src/glusterd-syncop.c +++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c @@ -245,11 +245,6 @@ glusterd_syncop_aggr_rsp_dict (glusterd_op_t op, dict_t *aggr, dict_t *rsp) } break; - case GD_OP_REPLACE_BRICK: - ret = glusterd_rb_use_rsp_dict (aggr, rsp); - if (ret) - goto out; - break; case GD_OP_SYNC_VOLUME: ret = glusterd_sync_use_rsp_dict (aggr, rsp); @@ -257,12 +252,6 @@ glusterd_syncop_aggr_rsp_dict (glusterd_op_t op, dict_t *aggr, dict_t *rsp) goto out; break; - case GD_OP_PROFILE_VOLUME: - ret = glusterd_profile_volume_use_rsp_dict (aggr, rsp); - if (ret) - goto out; - break; - case GD_OP_GSYNC_CREATE: break; @@ -278,12 +267,6 @@ glusterd_syncop_aggr_rsp_dict (glusterd_op_t op, dict_t *aggr, dict_t *rsp) goto out; break; - case GD_OP_REBALANCE: - case GD_OP_DEFRAG_BRICK_VOLUME: - ret = glusterd_volume_rebalance_use_rsp_dict (aggr, rsp); - if (ret) - goto out; - break; case GD_OP_HEAL_VOLUME: ret = glusterd_volume_heal_use_rsp_dict (aggr, rsp); diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 7347fb2..770cbf7 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -7404,13 +7404,11 @@ glusterd_rb_use_rsp_dict (dict_t *aggr, dict_t *rsp_dict) ctx = aggr; } else { - ctx = glusterd_op_get_ctx (); - if (!ctx) { - gf_msg ("glusterd", GF_LOG_ERROR, 0, - GD_MSG_OPCTX_GET_FAIL, - "Operation Context is not present"); - GF_ASSERT (0); - } + gf_msg (this->name, GF_LOG_ERROR, 0, + GD_MSG_OPCTX_GET_FAIL, + "Operation Context is not present"); + ret = -1; + goto out; } if (rsp_dict) { @@ -7518,14 +7516,15 @@ glusterd_profile_volume_use_rsp_dict (dict_t *aggr, dict_t *rsp_dict) ret = 0; //no bricks in the rsp goto out; } - - op = glusterd_op_get_op (); - GF_ASSERT (GD_OP_PROFILE_VOLUME == op); if (aggr) { ctx_dict = aggr; } else { - ctx_dict = glusterd_op_get_ctx (); + gf_msg ("glusterd", GF_LOG_ERROR, 0, + GD_MSG_OPCTX_GET_FAIL, + "Operation Context is not present"); + ret = -1; + goto out; } ret = dict_get_int32 (ctx_dict, "count", &count); @@ -7962,16 +7961,15 @@ glusterd_volume_rebalance_use_rsp_dict (dict_t *aggr, dict_t *rsp_dict) GF_ASSERT (rsp_dict); conf = THIS->private; - op = glusterd_op_get_op (); - GF_ASSERT ((GD_OP_REBALANCE == op) || - (GD_OP_DEFRAG_BRICK_VOLUME == op)); - if (aggr) { ctx_dict = aggr; } else { - ctx_dict = glusterd_op_get_ctx (op); - + gf_msg ("glusterd", GF_LOG_ERROR, 0, + GD_MSG_OPCTX_GET_FAIL, + "Operation Context is not present"); + ret = -1; + goto out; } if (!ctx_dict) diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index 27c51fa..44fcdd3 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -425,10 +425,6 @@ int32_t glusterd_handle_node_rsp (dict_t *req_ctx, void *pending_entry, glusterd_op_t op, dict_t *rsp_dict, dict_t *op_ctx, char **op_errstr, gd_node_type type); -int -glusterd_volume_rebalance_use_rsp_dict (dict_t *aggr, dict_t *rsp_dict); -int -glusterd_volume_heal_use_rsp_dict (dict_t *aggr, dict_t *rsp_dict); int32_t glusterd_check_if_quota_trans_enabled (glusterd_volinfo_t *volinfo); -- 1.7.1