提交 c9f2a816 编写于 作者: G ggbq 提交者: Hubert Zhang

Fix assertion failures in BackoffSweeper

Previous commit ab74e1c6, c7befb1d did not completely solve its race
condition, it did not test for last iteration of the while/for loop.
This could result in failed assertion in the following loop. The patch
moves the judgement to the ending of the for loop, it is safe, because
the first iteration will never trigger: Assert(activeWeight > 0.0).

Also, the other one race condition can trigger this assertion
Assert(gl->numFollowersActive > 0). Consider this situation:

    Backend A, B belong to the same statement.

    Timestamp1: backend A's leader is A, backend B's leader is B.

    Timestamp2: backend A's numFollowersActive remains zero due to timeout.

    Timestamp3: Sweeper calculates leader B's numFollowersActive to 1.

    Timestamp4: backend B changes it's leader to A even if A is inactive.

We stop sweeping for this race condition just like commit ab74e1c6 did.

Both Assert(activeWeight > 0.0) and Assert(gl->numFollowersActive > 0)
are removed.

(cherry picked from commit b1c19196)
上级 a3233b6b
......@@ -841,6 +841,7 @@ BackoffSweeper()
* backend can get */
Assert(maxCPU > 0.0);
Assert(activeWeight > 0.0);
if (gp_debug_resqueue_priority)
{
......@@ -864,37 +865,33 @@ BackoffSweeper()
double targetCPU = 0.0;
const BackoffBackendSharedEntry *gl = getBackoffEntryRO(se->groupLeaderIndex);
Assert(gl->numFollowersActive > 0);
if (activeWeight <= 0.0)
if (gl->numFollowersActive <= 0)
{
/*
* There is a race condition here:
* Backend A,B,C are belong to same statement and have weight of
* 100000.
*
* Timestamp1: backend A's leader is A, backend B's leader is B
* backend C's leader is also B.
* Backend A, B belong to the same statement. Backend A remains inactive
* longer than gp_resqueue_priority_inactivity_timeout.
*
* Timestamp1: backend A's leader is A, backend B's leader is B.
*
* Timestamp2: Sweeper calculates the activeWeight to 200000.
* Timestamp2: backend A's numFollowersActive remains zero due to timeout.
*
* Timestamp3: backend B changes it's leader to A.
* Timestamp3: Sweeper calculates leader B's numFollowersActive to 1.
*
* Timestamp4: Sweeper try to find the backends who deserve maxCPU,
* if backend A, B, C all deserve maxCPU, then activeWeight =
* 200000 - 100000/1 - 100000/1 - 100000/2 which is less than zero.
* Timestamp4: backend B changes it's leader to A.
*
* We can stop sweeping for such race condition because current
* backoff mechanism dose not ask for accurate control.
* Backend process can change the backoff group leader without checking whether
* the leader is an active backend due to performance consideration. This leads
* to a backend could switch to an inactive leader whose numFollowersActive is
* zero. Since backoff sweeper is not an accurate control, we could just skip
* it in the current loop.
*/
backoffSingleton->sweeperInProgress = false;
elog(LOG, "activeWeight underflow!");
elog(LOG, "numFollowersActive underflow!");
return;
}
Assert(activeWeight > 0.0);
Assert(se->weight > 0.0);
targetCPU = (CPUAvailable) * (se->weight) / activeWeight / gl->numFollowersActive;
/**
......@@ -912,6 +909,32 @@ BackoffSweeper()
CPUAvailable -= maxCPU;
found = true;
}
if (activeWeight <= 0.0)
{
/*
* There is a race condition here:
* Backend A,B,C belong to the same statement and have weight of
* 100000.
*
* Timestamp1: backend A's leader is A, backend B's leader is B
* backend C's leader is also B.
*
* Timestamp2: Sweeper calculates the activeWeight to 200000.
*
* Timestamp3: backend B changes it's leader to A.
*
* Timestamp4: Sweeper try to find the backends who deserve maxCPU,
* if backend A, B, C all deserve maxCPU, then activeWeight =
* 200000 - 100000/1 - 100000/1 - 100000/2 which is less than zero.
*
* We can stop sweeping for such race condition because current
* backoff mechanism dose not ask for accurate control.
*/
backoffSingleton->sweeperInProgress = false;
elog(LOG, "activeWeight underflow!");
return;
}
}
}
numIterations++;
......@@ -936,8 +959,6 @@ BackoffSweeper()
{
const BackoffBackendSharedEntry *gl = getBackoffEntryRO(se->groupLeaderIndex);
Assert(activeWeight > 0.0);
Assert(gl->numFollowersActive > 0);
Assert(se->weight > 0.0);
se->targetUsage = (CPUAvailable) * (se->weight) / activeWeight / gl->numFollowersActive;
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册