Skip to content

Commit

Permalink
rgw: civetweb should use unique request id
Browse files Browse the repository at this point in the history
max_req_id was moved to RGWRados and changed to atomic64_t.

The same request id resulted in gc giving the same idtag to all objects
resulting in a leakage of rados objects. It only kept the last deleted object in
it's queue, the previous objects were never freed.

Fixes: 10295
Backport: Hammer, Firefly

Signed-off-by: Orit Wasserman <owasserm@redhat.com>
  • Loading branch information
oritwas committed Apr 23, 2015
1 parent 2e8d476 commit c262259
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 14 deletions.
22 changes: 9 additions & 13 deletions src/rgw/rgw_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct RGWRequest
RGWOp *op;
utime_t ts;

RGWRequest() : id(0), s(NULL), op(NULL) {
RGWRequest(uint64_t id) : id(id), s(NULL), op(NULL) {
}

virtual ~RGWRequest() {}
Expand Down Expand Up @@ -158,7 +158,7 @@ struct RGWFCGXRequest : public RGWRequest {
FCGX_Request *fcgx;
QueueRing<FCGX_Request *> *qr;

RGWFCGXRequest(QueueRing<FCGX_Request *> *_qr) : qr(_qr) {
RGWFCGXRequest(uint64_t req_id, QueueRing<FCGX_Request *> *_qr) : RGWRequest(req_id), qr(_qr) {
qr->dequeue(&fcgx);
}

Expand Down Expand Up @@ -239,8 +239,6 @@ class RGWProcess {
}
} req_wq;

uint64_t max_req_id;

public:
RGWProcess(CephContext *cct, RGWProcessEnv *pe, int num_threads, RGWFrontendConfig *_conf)
: store(pe->store), olog(pe->olog), m_tp(cct, "RGWProcess::m_tp", num_threads),
Expand All @@ -249,8 +247,7 @@ class RGWProcess {
conf(_conf),
sock_fd(-1),
req_wq(this, g_conf->rgw_op_thread_timeout,
g_conf->rgw_op_thread_suicide_timeout, &m_tp),
max_req_id(0) {}
g_conf->rgw_op_thread_suicide_timeout, &m_tp) {}
virtual ~RGWProcess() {}
virtual void run() = 0;
virtual void handle_request(RGWRequest *req) = 0;
Expand Down Expand Up @@ -340,8 +337,7 @@ void RGWFCGXProcess::run()
}

for (;;) {
RGWFCGXRequest *req = new RGWFCGXRequest(&qr);
req->id = ++max_req_id;
RGWFCGXRequest *req = new RGWFCGXRequest(store->get_new_req_id(), &qr);
dout(10) << "allocated request req=" << hex << req << dec << dendl;
req_throttle.get(1);
int ret = FCGX_Accept_r(req->fcgx);
Expand Down Expand Up @@ -372,8 +368,8 @@ struct RGWLoadGenRequest : public RGWRequest {
atomic_t *fail_flag;


RGWLoadGenRequest(const string& _m, const string& _r, int _cl,
atomic_t *ff) : method(_m), resource(_r), content_length(_cl), fail_flag(ff) {}
RGWLoadGenRequest(uint64_t req_id, const string& _m, const string& _r, int _cl,
atomic_t *ff) : RGWRequest(req_id), method(_m), resource(_r), content_length(_cl), fail_flag(ff) {}
};

class RGWLoadGenProcess : public RGWProcess {
Expand Down Expand Up @@ -474,8 +470,8 @@ void RGWLoadGenProcess::run()

void RGWLoadGenProcess::gen_request(const string& method, const string& resource, int content_length, atomic_t *fail_flag)
{
RGWLoadGenRequest *req = new RGWLoadGenRequest(method, resource, content_length, fail_flag);
req->id = ++max_req_id;
RGWLoadGenRequest *req = new RGWLoadGenRequest(store->get_new_req_id(), method, resource,
content_length, fail_flag);
dout(10) << "allocated request req=" << hex << req << dec << dendl;
req_throttle.get(1);
req_wq.queue(req);
Expand Down Expand Up @@ -717,7 +713,7 @@ static int civetweb_callback(struct mg_connection *conn) {
RGWREST *rest = pe->rest;
OpsLogSocket *olog = pe->olog;

RGWRequest *req = new RGWRequest;
RGWRequest *req = new RGWRequest(store->get_new_req_id());
RGWMongoose client_io(conn, pe->port);

client_io.init(g_ceph_context);
Expand Down
7 changes: 6 additions & 1 deletion src/rgw/rgw_rados.h
Original file line number Diff line number Diff line change
Expand Up @@ -1188,6 +1188,7 @@ class RGWRados

void get_bucket_instance_ids(RGWBucketInfo& bucket_info, int shard_id, map<int, string> *result);

atomic64_t max_req_id;
Mutex lock;
Mutex watchers_lock;
SafeTimer *timer;
Expand Down Expand Up @@ -1249,7 +1250,7 @@ class RGWRados
Finisher *finisher;

public:
RGWRados() : lock("rados_timer_lock"), watchers_lock("watchers_lock"), timer(NULL),
RGWRados() : max_req_id(0), lock("rados_timer_lock"), watchers_lock("watchers_lock"), timer(NULL),
gc(NULL), use_gc_thread(false), quota_threads(false),
num_watchers(0), watchers(NULL),
watch_initialized(false),
Expand All @@ -1263,6 +1264,10 @@ class RGWRados
rest_master_conn(NULL),
meta_mgr(NULL), data_log(NULL) {}

uint64_t get_new_req_id() {
return max_req_id.inc();
}

void set_context(CephContext *_cct) {
cct = _cct;
}
Expand Down

0 comments on commit c262259

Please sign in to comment.