Yair Zaslavsky has posted comments on this change.
Change subject: engine: Refresh gluster data periodically
......................................................................
Patch Set 8: (11 inline comments)
....................................................
File backend/manager/modules/bll/pom.xml
Line 53: </dependency>
Line 54:
Line 55: <dependency>
Line 56: <groupId>${engine.groupId}</groupId>
Line 57: <artifactId>dal</artifactId>
Why does bll depend on this? Does it relate to this patch or should be in a
different patch?
Line 58: <version>${engine.version}</version>
Line 59: <type>test-jar</type>
Line 60: <scope>test</scope>
Line 61: </dependency>
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterManager.java
Line 199: * @return
Line 200: */
Line 201: private boolean isRemovableStatus(VDSStatus status) {
Line 202: switch (status) {
Line 203: case Up:
Bad indentation, please fix.
Line 204: case Down:
Line 205: case Maintenance:
Line 206: return true;
Line 207: default:
Line 228: }
Line 229:
Line 230: private List<String> getVdsIps(VDS vds) {
Line 231: List<String> vdsIps = new ArrayList<String>();
Line 232: for (VdsNetworkInterface iface :
getInterfaceDao().getAllInterfacesForVds(vds.getId())) {
Use addAll, and not this for.
Line 233: vdsIps.add(iface.getAddress());
Line 234: }
Line 235: return vdsIps;
Line 236: }
Line 386: for (GlusterVolumeEntity volume :
getVolumeDao().getByClusterId(clusterId)) {
Line 387: if (!volumesMap.containsKey(volume.getName())) {
Line 388: log.debugFormat("Volume {0} has been removed directly
using the gluster CLI. Removing it from engine as well.",
Line 389: volume.getName());
Line 390: getVolumeDao().remove(volume.getId());
Consider using the MassOperationsDAO to peform X remove operations on a single
DB connection.
Line 391: logVolumeMessage(volume,
AuditLogType.GLUSTER_VOLUME_DELETED_FROM_CLI);
Line 392: }
Line 393: }
Line 394: }
Line 393: }
Line 394: }
Line 395:
Line 396: private void updateExistingAndNewVolumes(Guid clusterId,
Map<String, GlusterVolumeEntity> volumesMap) {
Line 397: for (Entry<String, GlusterVolumeEntity> entry :
volumesMap.entrySet()) {
Consider using the MassOperationsDAO to perform X remove operations on a single
DB connection.
Line 398: GlusterVolumeEntity volume = entry.getValue();
Line 399: log.debugFormat("Analyzing volume {0}", volume.getName());
Line 400:
Line 401: GlusterVolumeEntity existingVolume =
getVolumeDao().getByName(clusterId, volume.getName());
Line 523: if (!GlusterCoreUtil.containsBrick(fetchedBricks,
existingBrick)) {
Line 524: log.infoFormat("Brick {0} removed from volume {1}
from CLI. Removing it from engine DB as well.",
Line 525: existingBrick.getQualifiedName(),
Line 526: existingVolume.getName());
Line 527: getBrickDao().removeBrick(existingBrick.getId());
Consider using MassOperationsDAO pattern to use single connection for removal.
Line 528: logAuditMessage(existingVolume.getClusterId(),
existingVolume, null,
Line 529:
AuditLogType.GLUSTER_VOLUME_BRICK_REMOVED_FROM_CLI,
Line 530: ENTITY_BRICK,
Line 531: existingBrick.getQualifiedName());
Line 577: if (fetchedVolume.getOption(existingOption.getKey()) ==
null) {
Line 578: log.infoFormat("Option {0} unset on volume {1} from
CLI. Removing it from engine DB as well.",
Line 579: existingOption.getKey(),
Line 580: fetchedVolume.getName());
Line 581:
getOptionDao().removeVolumeOption(existingOption.getId());
Same as before.
Line 582: logAuditMessage(fetchedVolume.getClusterId(),
fetchedVolume, null,
Line 583:
AuditLogType.GLUSTER_VOLUME_OPTION_RESET_FROM_CLI,
Line 584: ENTITY_OPTION,
Line 585: existingOption.getKey());
Line 588: }
Line 589:
Line 590: private void updateExistingAndNewOptions(GlusterVolumeEntity
existingVolume,
Line 591: Collection<GlusterVolumeOptionEntity> fetchedOptions) {
Line 592: for (GlusterVolumeOptionEntity fetchedOption :
fetchedOptions) {
Same as before
Line 593: GlusterVolumeOptionEntity existingOption =
existingVolume.getOption(fetchedOption.getKey());
Line 594: if (existingOption == null) {
Line 595: log.infoFormat("New option {0}={1} set on volume {2}
from gluster CLI. Updating engine DB accordingly.",
Line 596: fetchedOption.getKey(),
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitBackendServicesOnStartupBean.java
Line 54: });
Line 55: StoragePoolStatusHandler.Init();
Line 56:
Line 57: GlusterManager.getInstance().init();
Line 58: log.infoFormat("GlusterManager: {0}", new Date());
This log should be at the scope GlusterManager.
I know that line 50, 52, 37, and 40 present different behavior - I think they
should be fixed (i.e - log of init should be a part of the init method).
Line 59: }
Line 60:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetNonOperationalVdsCommand.java
Line 75: for (Entry<String, String> e :
getParameters().getCustomLogValues().entrySet()) {
Line 76: AddCustomValue(e.getKey(), e.getValue());
Line 77: }
Line 78: switch (getParameters().getNonOperationalReason()) {
Line 79: case NETWORK_UNREACHABLE:
Wrong indentation, please fix.
Line 80: return (getSucceeded()) ?
AuditLogType.VDS_SET_NONOPERATIONAL_NETWORK
Line 81: : AuditLogType.VDS_SET_NONOPERATIONAL_FAILED;
Line 82: case STORAGE_DOMAIN_UNREACHABLE:
Line 83: return (getSucceeded()) ?
AuditLogType.VDS_SET_NONOPERATIONAL_DOMAIN
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 199: GLUSTER_VOLUME_REPLACE_BRICK_START_FAILED(4018),
Line 200: GLUSTER_VOLUME_ADD_BRICK(4019),
Line 201: GLUSTER_VOLUME_ADD_BRICK_FAILED(4020),
Line 202: GLUSTER_HOST_REMOVE_FAILED(4021),
Line 203: GLUSTER_VOLUME_CREATED_FROM_CLI(4022),
What CLI is this?
Line 204: GLUSTER_VOLUME_DELETED_FROM_CLI(4023),
Line 205: GLUSTER_VOLUME_OPTION_SET_FROM_CLI(4024),
Line 206: GLUSTER_VOLUME_OPTION_RESET_FROM_CLI(4025),
Line 207: GLUSTER_VOLUME_PROPERTIES_CHANGED_FROM_CLI(4026),
--
To view, visit http://gerrit.ovirt.org/7288
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6b61eb6e93105e46e2706eac1d94bc10717224c2
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
