[WIP] Reorder disks if mounts have been shuffled. #2931
Conversation
litingulfs
force-pushed
the
tingulfs/reorderdisks
branch
from
4be1491
to
b25dcb9
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2931 +/- ##
=============================================
- Coverage 64.24% 31.45% -32.79%
+ Complexity 10398 5432 -4966
=============================================
Files 840 867 +27
Lines 71755 73754 +1999
Branches 8611 8883 +272
=============================================
- Hits 46099 23201 -22898
- Misses 23004 48730 +25726
+ Partials 2652 1823 -829 ☔ View full report in Codecov by Sentry. |
litingulfs
force-pushed
the
tingulfs/reorderdisks
branch
from
b25dcb9
to
0465eca
Compare
| */ | ||
| private Set<String> findPartitionsOnDisk(DiskId disk) { | ||
| Set<String> partitionDirs = new HashSet<>(); | ||
| File[] directories = new File(disk.getMountPath()).listFiles(File::isDirectory); |
There was a problem hiding this comment.
We should probably check if the disk.getMountPath() exists or not. List file on nonexisting directory would throw some exception.
| if(!foundDiskToPartitionMap.get(currentDisk).contains(partitionID)) { | ||
| brokenDisks.add(currentDisk); | ||
| } |
There was a problem hiding this comment.
If a host got reimaged, and came back with empty disks, then we don't want to think those disks are broken.
| if(foundDisk == null) { | ||
| return Collections.emptyMap(); | ||
| } else { | ||
| shuffledDisks.put(currentDisk, foundDisk); | ||
| } |
There was a problem hiding this comment.
The empty disk might be covered by these statements.
| if (!dataNodeConfigSource.set(dataNodeConfig)) { | ||
| logger.error("Setting disks order failed DataNodeConfig update"); | ||
| success = false; | ||
| } |
There was a problem hiding this comment.
we should only call dataNodeConfigSource.set once, after for loop.
|
|
||
| // Swap the disks in the DataNodeConfig | ||
| logger.info("Replacing disk {} with disk {}", oldDisk.getMountPath(), newDisk.getMountPath()); | ||
| dataNodeConfig.getDiskConfigs().put(oldDisk.getMountPath(), dataNodeConfig.getDiskConfigs().get(newDisk)); |
There was a problem hiding this comment.
this is probably not enough, say disk1 shuffles to disk2 and disk2 shuffle to disk1.
In this case, the newDiskMapping would have {disk1:disk2, disk2:disk1}.
Say we are dealing with disk1:disk2 entry and we are setting disk1 (old) to disk2's config. Now the dataNodeConfig's disk config map would look like {disk1:disk2config, disk2:disk2config}. Disk1config is missing.
I think we have to make a copy of dataNodeConfig's disk config map and use this copied map as temporary reference.
No description provided.