Skip to content

Commit

Permalink
pool: Fix accounting error in repository statistics
Browse files Browse the repository at this point in the history
Motivation:

We pre-calculate the output of 'rep ls -s' by listening for repository
change events. We have observed this pre-calculation to claim that we
have a negative number of files with a sticky flag.

The repository generates change events when sticky flags expire. The change
event is generated for each expired sticky flag, however the before and after
state inluded in the event is the same for all these notifications. For the
code maintaining the repository statistics this is fatal as it will observe the
same file changing from sticky to non-sticky several times even when reality
two sticky flags were expired in one operation.

Modification:

Do not include the sticky flag that changed in the notification; it was not
used by any listener and if needed it can be extracted by calculsting the
difference of the before and after state.

Result:

Fixed an accounting bug that could cause the output of 'rep ls -s' to be
wrong.

Target: trunk
Request: 2.14
Request: 2.13
Request: 2.12
Require-notes: yes
Require-book: no
Acked-by: Tigran Mkrtchyan <tigran.mkrtchyan@desy.de>
Patch: https://rb.dcache.org/r/8925/
(cherry picked from commit 830070e6630db10ae2e35fff3998f66de3cfe3d1)
  • Loading branch information
gbehrmann committed Jan 13, 2016
1 parent 0a48e12 commit 6ba57be
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 23 deletions.
Expand Up @@ -6,28 +6,13 @@
*/
public class StickyChangeEvent extends EntryChangeEvent
{
private final StickyRecord _sticky;

public StickyChangeEvent(CacheEntry oldEntry, CacheEntry newEntry, StickyRecord sticky)
public StickyChangeEvent(CacheEntry oldEntry, CacheEntry newEntry)
{
super(oldEntry, newEntry);
_sticky = sticky;
}

/**
* Returns the state of the change sticky record after the
* update. Any sticky record with an expiration time in the past
* should be considered removed from the entry.
*/
public StickyRecord getStickyRecord()
{
return _sticky;
}

public String toString()
{
return
String.format("StickyRecordChangeEvent [id=%s,sticky=%s]",
getPnfsId(), _sticky);
return String.format("StickyRecordChangeEvent [id=%s]", getPnfsId());
}
}
Expand Up @@ -691,7 +691,7 @@ public void setSticky(PnfsId id, String owner,
CacheEntry oldEntry = new CacheEntryImpl(entry);
if (entry.setSticky(owner, expire, overwrite) && _state == State.OPEN) {
CacheEntryImpl newEntry = new CacheEntryImpl(entry);
stickyChanged(oldEntry, newEntry, new StickyRecord(owner, expire));
stickyChanged(oldEntry, newEntry);
scheduleExpirationTask(entry);
}
} catch (CacheException e) {
Expand Down Expand Up @@ -939,10 +939,10 @@ protected void accessTimeChanged(CacheEntry oldEntry, CacheEntry newEntry)
* record.
*/
@GuardedBy("getMetaDataRecord(newEntry.getPnfsid())")
protected void stickyChanged(CacheEntry oldEntry, CacheEntry newEntry, StickyRecord record)
protected void stickyChanged(CacheEntry oldEntry, CacheEntry newEntry)
{
updateRemovable(newEntry);
StickyChangeEvent event = new StickyChangeEvent(oldEntry, newEntry, record);
StickyChangeEvent event = new StickyChangeEvent(oldEntry, newEntry);
_stateChangeListeners.stickyChanged(event);
}

Expand Down Expand Up @@ -1015,7 +1015,7 @@ void setSticky(MetaDataRecord entry, String owner,
CacheEntry oldEntry = new CacheEntryImpl(entry);
if (entry.setSticky(owner, expire, overwrite) && _state == State.OPEN) {
CacheEntryImpl newEntry = new CacheEntryImpl(entry);
stickyChanged(oldEntry, newEntry, new StickyRecord(owner, expire));
stickyChanged(oldEntry, newEntry);
scheduleExpirationTask(entry);
}
}
Expand Down Expand Up @@ -1112,9 +1112,9 @@ private void removeExpiredStickyFlags(MetaDataRecord entry) throws CacheExceptio
synchronized (entry) {
CacheEntry oldEntry = new CacheEntryImpl(entry);
Collection<StickyRecord> removed = entry.removeExpiredStickyFlags();
for (StickyRecord record: removed) {
if (!removed.isEmpty()) {
CacheEntryImpl newEntry = new CacheEntryImpl(entry);
stickyChanged(oldEntry, newEntry, record);
stickyChanged(oldEntry, newEntry);
}
scheduleExpirationTask(entry);
}
Expand Down

0 comments on commit 6ba57be

Please sign in to comment.