[ https://issues.apache.org/jira/browse/GEODE-3258?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16117417#comment-16117417 ]
ASF GitHub Bot commented on GEODE-3258: --------------------------------------- Github user pdxrunner commented on a diff in the pull request: https://github.com/apache/geode/pull/687#discussion_r131777554 --- Diff: geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/CompactDiskStoreCommand.java --- @@ -0,0 +1,185 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ + +package org.apache.geode.management.internal.cli.commands; + +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.springframework.shell.core.annotation.CliCommand; +import org.springframework.shell.core.annotation.CliOption; + +import org.apache.geode.cache.persistence.PersistentID; +import org.apache.geode.distributed.DistributedMember; +import org.apache.geode.distributed.internal.InternalDistributedSystem; +import org.apache.geode.distributed.internal.membership.InternalDistributedMember; +import org.apache.geode.internal.cache.InternalCache; +import org.apache.geode.management.DistributedSystemMXBean; +import org.apache.geode.management.ManagementService; +import org.apache.geode.management.cli.CliMetaData; +import org.apache.geode.management.cli.ConverterHint; +import org.apache.geode.management.cli.Result; +import org.apache.geode.management.internal.cli.CliUtil; +import org.apache.geode.management.internal.cli.LogWrapper; +import org.apache.geode.management.internal.cli.i18n.CliStrings; +import org.apache.geode.management.internal.cli.result.CompositeResultData; +import org.apache.geode.management.internal.cli.result.ResultBuilder; +import org.apache.geode.management.internal.messages.CompactRequest; +import org.apache.geode.management.internal.security.ResourceOperation; +import org.apache.geode.security.ResourcePermission; + +public class CompactDiskStoreCommand implements GfshCommand { + @CliCommand(value = CliStrings.COMPACT_DISK_STORE, help = CliStrings.COMPACT_DISK_STORE__HELP) + @CliMetaData(relatedTopic = {CliStrings.TOPIC_GEODE_DISKSTORE}) + @ResourceOperation(resource = ResourcePermission.Resource.CLUSTER, + operation = ResourcePermission.Operation.MANAGE, target = ResourcePermission.Target.DISK) + public Result compactDiskStore( + @CliOption(key = CliStrings.COMPACT_DISK_STORE__NAME, mandatory = true, + optionContext = ConverterHint.DISKSTORE, + help = CliStrings.COMPACT_DISK_STORE__NAME__HELP) String diskStoreName, + @CliOption(key = {CliStrings.GROUP, CliStrings.GROUPS}, + help = CliStrings.COMPACT_DISK_STORE__GROUP__HELP) String[] groups) { + Result result; + + try { + // disk store exists validation + if (!diskStoreExists(diskStoreName)) { + result = ResultBuilder.createUserErrorResult( + CliStrings.format(CliStrings.COMPACT_DISK_STORE__DISKSTORE_0_DOES_NOT_EXIST, + new Object[] {diskStoreName})); + } else { + InternalDistributedSystem ds = getCache().getInternalDistributedSystem(); + + Map<DistributedMember, PersistentID> overallCompactInfo = new HashMap<>(); + + Set<?> otherMembers = ds.getDistributionManager().getOtherNormalDistributionManagerIds(); + Set<InternalDistributedMember> allMembers = new HashSet<>(); + + for (Object member : otherMembers) { + allMembers.add((InternalDistributedMember) member); + } + allMembers.add(ds.getDistributedMember()); + + String groupInfo = ""; + // if groups are specified, find members in the specified group + if (groups != null && groups.length > 0) { + groupInfo = CliStrings.format(CliStrings.COMPACT_DISK_STORE__MSG__FOR_GROUP, + new Object[] {Arrays.toString(groups) + "."}); + final Set<InternalDistributedMember> selectedMembers = new HashSet<>(); + List<String> targetedGroups = Arrays.asList(groups); + for (InternalDistributedMember member : allMembers) { + List<String> memberGroups = member.getGroups(); + if (!Collections.disjoint(targetedGroups, memberGroups)) { + selectedMembers.add(member); + } + } + + allMembers = selectedMembers; + } + + // allMembers should not be empty when groups are not specified - it'll + // have at least one member + if (allMembers.isEmpty()) { + result = ResultBuilder.createUserErrorResult( + CliStrings.format(CliStrings.COMPACT_DISK_STORE__NO_MEMBERS_FOUND_IN_SPECIFED_GROUP, + new Object[] {Arrays.toString(groups)})); + } else { + // first invoke on local member if it exists in the targeted set + if (allMembers.remove(ds.getDistributedMember())) { + PersistentID compactedDiskStoreId = CompactRequest.compactDiskStore(diskStoreName); + if (compactedDiskStoreId != null) { + overallCompactInfo.put(ds.getDistributedMember(), compactedDiskStoreId); + } + } + + // was this local member the only one? Then don't try to send + // CompactRequest. Otherwise, send the request to others + if (!allMembers.isEmpty()) { + // Invoke compact on all 'other' members + Map<DistributedMember, PersistentID> memberCompactInfo = + CompactRequest.send(ds.getDistributionManager(), diskStoreName, allMembers); + if (memberCompactInfo != null && !memberCompactInfo.isEmpty()) { + overallCompactInfo.putAll(memberCompactInfo); + memberCompactInfo.clear(); + } + String notExecutedMembers = CompactRequest.getNotExecutedMembers(); + if (notExecutedMembers != null && !notExecutedMembers.isEmpty()) { + LogWrapper.getInstance() + .info("compact disk-store \"" + diskStoreName + + "\" message was scheduled to be sent to but was not send to " --- End diff -- I realize this goes beyond a simple class-extraction refactor, this message is poorly worded. A couple alternatives: "message was scheduled to be sent but was not for " or "message was scheduled to be sent to but was not sent to " > Refactor DiskStoreCommands > -------------------------- > > Key: GEODE-3258 > URL: https://issues.apache.org/jira/browse/GEODE-3258 > Project: Geode > Issue Type: Sub-task > Components: gfsh > Reporter: Emily Yeh > Assignee: Emily Yeh > > {{DiskStoreCommands.java}} is a large class that contains multiple commands. > Each command should be refactored into a separate class, and the methods > shared by the commands should be refactored into a new and appropriately > named class of their own. -- This message was sent by Atlassian JIRA (v6.4.14#64029)