Paths

Table of Contentst

Differential D16492

Ignore repository versions on inactive devices in "Repository Servers" panel in Config
ClosedPublic
Actions

Authored by epriestley on Sep 5 2016, 5:39 PM.

Details

Reviewers

chad

Maniphest Tasks

T11590: "Repository Servers" panel in Config incorrectly displaying out-of-date leader information?

Commits

rP4dc37bcee080: Ignore repository versions on inactive devices in "Repository Servers" panel in…

Summary

Fixes T11590. Currently, we incorrectly consider cluster repository versions that are (or were) on devices which are no longer part of the active cluster service when building this status screen.

Instead, ignore them. This is just a display bug; the actual ClusterEngine already had similar logic.

Test Plan

Added a bad leader record to repository_workingcopyversion.
Before patch, got a bad "Partial (1w)" sync:

After patch, got a good "Sycnchronized":

Diff Detail

Repository

rP Phabricator

Branch

clusterleader

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 13569
Build 17473: Run Core Tests
Build 17472: arc lint + arc unit

Event Timeline

epriestley updated this revision to Diff 39689.Sep 5 2016, 5:39 PM

epriestley retitled this revision from to Ignore repository versions on inactive devices in "Repository Servers" panel in Config.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T11590: "Repository Servers" panel in Config incorrectly displaying out-of-date leader information?.

This is just a display bug; the actual ClusterEngine already had similar logic.

This is not completely true: ClusterEngine is correct and this is just a display bug, but the ClusterEngine logic isn't that similar:

After a hosting change (observed to hosted or vice versa), we reset all the version clocks. This is fine/reasonable.
When synchronizing before a read, we examine all versions, but this is correct since we should fail if the leader somehow got deactivated.
When synchronizing before a write, we examine all versions, but this is correct since we should fail even if a deactivated node is holding a write lock.
We show all versions in Diffusion → Manage Repository → Storage, but properly mark disabled nodes as disabled.

chad accepted this revision.Sep 5 2016, 6:06 PM

chad edited edge metadata.

This revision is now accepted and ready to land.Sep 5 2016, 6:06 PM

Also fix a "cluter" -> "cluster" typo.

Closed by commit rP4dc37bcee080: Ignore repository versions on inactive devices in "Repository Servers" panel in… (authored by epriestley, committed by epriestley). · Explain WhySep 5 2016, 6:10 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

applications/

config/

controller/

PhabricatorConfigClusterRepositoriesController.php

8 lines

diffusion/

protocol/

DiffusionRepositoryClusterEngine.php

2 lines

Diff 39690

View Options

src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php