MySQL Bugs: #42505: Falcon: Record backlogging not enabled by the Scavenger

Bug #42505	Falcon: Record backlogging not enabled by the Scavenger
Submitted:	1 Feb 0:22	Modified:	27 Feb 20:24
Reporter:	Christopher Powers
Status:	Patch queued
Category:	Server: Falcon	Severity:	S2 (Serious)
Version:	6.0-falcon-team	OS:	Any
Assigned to:	Christopher Powers	Target Version:	6.0.10
Tags:	Memory, scavenger, F_BACKLOG
Triage:	Triaged: D3 (Medium)

Description:
The Scavenger fails to enable the backlogging facility when the available record cache
memory drops below the record cache floor.

Additionally, the Backlog class uses a tablespace id of -1 which hashes to the -1 slot in
SerialLog::tableSpaces[]. From a C++ standpoint, negative array subscripts are not
illegal, but the array is not sized to accommodate elements below 0, so a tablespace id
of -1 can result in memory corruption.

Finally, once the Backlog is enabled it is never disabled. This, too, should be fixed.

How to repeat:
Set these parameters:

falcon_record_memory_max = 100 MB,
falcon_record_chill_threshold = 2,

Run the testcase from Bug#36442 or any testcase that fills the record cache beyond what
the scavenger can handle.

Notes:

1. With the current implementation, backlogging is *enabled* by the Scavenger, but is
*triggered* by record chilling.

2. Backlogging is enabled when the Scavenger sets a low memory condition. This occurs
when the Scavenger cannot reduce the record cache memory below falcon_record_memory_max *
falcon_record_scavenge_threshold/100.

3. When backlogging is enabled, a message will appear in the Falcon record console.

4. Backlogging is triggered by record chilling during a low memory condition..

5. When backlogging is triggered, backlogging messages will appear in the Falcon
console.

Currently, record backlogging is very tricky to test. I will post a backlog test script
once backlogging is fixed.

Suggested fix:
1. Use the sequence manager to assign a tablespace id to the Backlogger.

2. Use the correct Scavenger memory totals to enable Backlogging.

3. Disable Backlogging when the record cache settles to a nominal level.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/64759

2990 Christopher Powers	2009-01-31
      Bug #42505, "Falcon: Record backlogging not enabled by the Scavenger"
      
      Fixed several issues with record backlogging.

This patch implements the first two suggested fixes forthis bug.  The third suggestion,
turn off backlogging, is not implemented.  Chris should decide if this change is complete
or if a new bug should be opened to improve backlogging more.

Note that by comparing 'if (recordScavenge.retiredActiveMemory > recordScavengeFloor)',
this patch uses a moving target.  This is because retiredActiveMemory is the actual
memory after the scavenge cycle.  This actual memory level went down during the scavenge
while also going up because of new records allocated.  It does not necessarily reflect
how successful the scavenge cycle was.  In fact, this test will almost always succeed
after the first scavenge cycle that is over the threshold.  The cycle will only try to
move the active memory sown to the floor.  But if any records are allocated at all,
chances are, retiredActiveMemory will be higher than the floor.  

I think the following code should be removed from RecordVersion::retire()

-	// Cannot retire this record.  Add up remaining space.
-
-	recordScavenge->recordsRemaining++;
-	recordScavenge->spaceRemaining += getMemUsage();
-
-	for (Record *prior = getPriorVersion(); prior; prior = prior->getPriorVersion())
-		{
-		++recordScavenge->recordsRemaining;
-		recordScavenge->spaceRemaining += prior->getMemUsage();
-		}

Then spaceRemaining should be calcuated based on just how much was actually done during
the cycle.  Oprofile results have shown a surprising amount of time spent calling
MemMgr::getMemUsage().  The calls above are not needed when it can be calculated as
below.

-	recordsRemaining = totalRecords - recordsPruned;
+	recordsRemaining = totalRecords - recordsPruned - recordsRetired;
-	spaceRemaining = totalRecordSpace - spacePruned;
+	spaceRemaining = totalRecordSpace - spacePruned - spaceRetired;

If the above is implemented, the following change from the patch will not be needed;
@@ -1816,7 +1816,7 @@ void Database::scavengeRecords(void)
-	if (recordScavenge.spaceRemaining > recordScavengeFloor)
+	if (recordScavenge.retiredActiveMemory > recordScavengeFloor)
 		setLowMemory();

Then the code will be faster, without making a bunch of calls to getMemUsage(), and it
would be more accurate to turn backlogging on.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/64959

2995 Christopher Powers	2009-02-02
      Bug #42505 "Falcon: Record backlogging not enabled by the Scavenger"
      
      Limit updateCardinalities() to once every 20 scavenge cycles. This
      function is too expensive to call every scavenge cycle. 
      
      Made Changes per Kevin's comments in the bug report, with one
      exception: Database::scavengeRecords() still determines low
      memory with retiredActiveMemory > recordScavengeFloor. Using 
      spaceRemaining > recordScavengeFloor does not enable backlogging
      when needed (confirmed against falcon_bug_36294-big.test).

There are a number of places in the code that treat the backlog
tablespace differently based on its illegal number.  Do you really
need to change that?

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/65282

2999 Christopher Powers	2009-02-04
      Bug #42505 "Falcon: Record backlogging not enabled by the Scavenger"
      
      Use negative tablespace ids for reserved tablespaces

This solution works to keep the negative tablespaceID for the backlog tablespace that
'other code' relies on.  And it has a slot within the hash table.  New patch is also
approved.

Pushed into 6.0.10-alpha (revid:alik@sun.com-20090211182317-uagkyj01fk30p1f8) (version
source revid:christopher.powers@sun.com-20090205053442-3d2okbitpbc5zx1q) (merge vers:
6.0.10-alpha) (pib:6)