Permalink
Browse files

Meta server: remove unnecessary and potentially harmful permission ch…

…eck optimization with append allocations. One problem with is that the -EPERM returned by AllocateChunkForAppend() can potentially clash with -1 returned in other cases. The other problem is that the cached permissions in the record append cache can become stale / out of date. On the other hand this optimization has practically no benefit as it only affects the "permissions denied" case, and has no effect on the successful allocations.
  • Loading branch information...
1 parent fb5ff0f commit 8a7b1efe59cd0e68569625ae6f2723d7ab2b4184 @mikeov mikeov committed Feb 16, 2013
Showing with 3 additions and 19 deletions.
  1. +1 −6 src/cc/meta/LayoutManager.cc
  2. +1 −4 src/cc/meta/LayoutManager.h
  3. +1 −7 src/cc/meta/MetaRequest.cc
  4. +0 −2 src/cc/meta/MetaRequest.h
@@ -250,8 +250,7 @@ ARAChunkCache::RequestNew(MetaAllocate& req)
req.chunkVersion,
req.offset,
TimeNow(),
- last,
- req.permissions
+ last
);
}
@@ -4829,10 +4828,6 @@ LayoutManager::AllocateChunkForAppend(MetaAllocate* req)
mARAChunkCache.Invalidate(req->fid);
return -1;
}
- if (mVerifyAllOpsPermissionsFlag && ! entry->permissions.CanWrite(
- req->euser, req->egroup)) {
- return -EACCES;
- }
// The client is providing an offset hint in the case when it needs a
// new chunk: space allocation failed because chunk is full, or it can
// not talk to the chunk server.
@@ -496,8 +496,7 @@ class ARAChunkCache
seq_t cv = -1,
chunkOff_t co = -1,
time_t now = 0,
- MetaAllocate* req = 0,
- Permissions perms = Permissions())
+ MetaAllocate* req = 0)
: chunkId(cid),
chunkVersion(cv),
offset(co),
@@ -506,7 +505,6 @@ class ARAChunkCache
spaceReservationSize(0),
numAppendersInChunk(0),
master(req ? req->master : ChunkServerPtr()),
- permissions(perms),
lastPendingRequest(req),
responseStr()
{}
@@ -527,7 +525,6 @@ class ARAChunkCache
// # of appenders to which this chunk was used for allocation
int numAppendersInChunk;
ChunkServerPtr master;
- Permissions permissions;
private:
MetaAllocate* lastPendingRequest;
string responseStr;
@@ -1477,16 +1477,12 @@ MetaAllocate::handle()
logFlag = false; // Do not emit redundant log record.
return;
}
- if (status == -EACCES) {
- return;
- }
offset = -1; // Allocate a new chunk past eof.
}
// force an allocation
chunkId = 0;
initialChunkVersion = -1;
vector<MetaChunkInfo*> chunkBlock;
- MetaFattr* fa = 0;
// start at step #2 above.
status = metatree.allocateChunkId(
fid, offset,
@@ -1498,14 +1494,12 @@ MetaAllocate::handle()
&chunkBlockStart,
gLayoutManager.VerifyAllOpsPermissions() ?
euser : kKfsUserRoot,
- egroup,
- &fa
+ egroup
);
if (status != 0 && (status != -EEXIST || appendChunk)) {
// we have a problem
return;
}
- permissions = *fa;
if (stripedFileFlag && appendChunk) {
status = -EINVAL;
statusMsg = "append is not supported with striped files";
@@ -883,7 +883,6 @@ struct MetaAllocate: public MetaRequest, public KfsCallbackObj {
MetaAllocate* next;
int64_t leaseId;
chunkOff_t chunkBlockStart;
- Permissions permissions;
MetaLeaseRelinquish* pendingLeaseRelinquish;
string responseStr; // Cached response
// With StringBufT instead of string the append allocation (presently
@@ -913,7 +912,6 @@ struct MetaAllocate: public MetaRequest, public KfsCallbackObj {
next(0),
leaseId(-1),
chunkBlockStart(-1),
- permissions(),
pendingLeaseRelinquish(0),
responseStr(),
clientHost(),

0 comments on commit 8a7b1ef

Please sign in to comment.