Project

General

Profile

Bug #2306

objecter: accessing empty object maps to pool 0

Added by Yehuda Sadeh almost 12 years ago. Updated almost 12 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

Even if different pool was specified.

History

#1 Updated by Yehuda Sadeh almost 12 years ago

Empty object <== object with empty name

#2 Updated by Greg Farnum almost 12 years ago

  • Status changed from New to Fix Under Review
  • Assignee set to Greg Farnum

Yep, the Objecter doesn't calculate pg placement for objects with a zero-length name. I'm pretty sure the if guard there is just completely incorrect — try the HEAD on my wip-misc-fixes branch; that should fix it.

#3 Updated by Sage Weil almost 12 years ago

i think that if was there for the pg ops (PGLS) where there is no object... the list_objects code is filling in the pgid explicitly, i think.

#4 Updated by Greg Farnum almost 12 years ago

Ah, you're right. I missed that function when looking to see who filled in the op->pgid.

In that case we should maybe be checking if the op->pgid is default-initialized? I don't know if our hash algorithms can output zero for the placement seed or not.

Otherwise we'll need to add a flag "explictly calculated target" that the listing code can set, or guard for empty object names farther up in the code hierarchy. (That might be better, actually, although I hate to clutter things up that way.)

#5 Updated by Yehuda Sadeh almost 12 years ago

From what I see, the pg ops call pool_op_submit() and not op_submit() so Greg's fix might be ok?

#6 Updated by Yehuda Sadeh almost 12 years ago

Ah, nope. list_objects is broken.

#7 Updated by Sage Weil almost 12 years ago

i prefer an explicit separate field for oid-vs-pg mode so that we can distinguish between pg 0.0 (really) and no pg/not initialized.

#8 Updated by Yehuda Sadeh almost 12 years ago

Would something like this work (not tested)?

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 24b95d2..acd4ef7 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -974,7 +974,7 @@ int Objecter::recalc_op_target(Op *op)
 {
   vector<int> acting;
   pg_t pgid = op->pgid;
-  if (op->oid.name.length()) {
+  if (!op->precalc_pgid) {
     int ret = osdmap->object_locator_to_pg(op->oid, op->oloc, pgid);
     if (ret == -ENOENT)
       return RECALC_OP_TARGET_POOL_DNE;
@@ -1349,6 +1349,7 @@ void Objecter::list_objects(ListContext *list_context, Context *onfinish) {
   o->reply_epoch = &onack->epoch;

   o->pgid = pg_t(list_context->current_pg, list_context->pool_id, -1);
+  o->precalc_pgid = true;

   op_submit(o);
 }
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index e733d33..d90e314 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -598,6 +598,8 @@ public:

     utime_t stamp;

+    bool precalc_pgid;
+
     Op(const object_t& o, const object_locator_t& ol, vector<OSDOp>& op,
        int f, Context *ac, Context *co, eversion_t *ov) :
       session(NULL), session_item(this), incarnation(0),
@@ -607,7 +609,7 @@ public:
       outbl(NULL),
       flags(f), priority(0), onack(ac), oncommit(co),
       tid(0), attempts(0),

-      paused(false), objver(ov), reply_epoch(NULL) {
+      paused(false), objver(ov), reply_epoch(NULL), precalc_pgid(false) {
       ops.swap(op);

       /* initialize out_* to match op vector */

#9 Updated by Greg Farnum almost 12 years ago

  • Assignee changed from Greg Farnum to Yehuda Sadeh

Yep, that's pretty much exactly what I was thinking.

The only other question is if this fix is the right approach — what will the rest of the system do with empty object names? Does it care? Should we forbid them as a safety measure and let one of the Objecter functions reject any attempt to use an empty object name?

#10 Updated by Sage Weil almost 12 years ago

that looks right to me.

and yeah, i don't think object operations should be possible on an empty object name...

#11 Updated by Sage Weil almost 12 years ago

  • Status changed from Fix Under Review to Resolved

Also available in: Atom PDF