Skip to content

Commit

Permalink
[PLAT-13438]: Add support for preview gflags
Browse files Browse the repository at this point in the history
Summary:
Added support for preview gFlags in YBA.
As part of this change,
 -  We have added validation in BE to ensure that the user should set the preview flag name in the allowed_preview_flags_csv flag.
 -  Added changes in the validate gflags API used by UI to catch this issue earlier in UI, even before the actual upgrade trigger.
 -  While setting the gflags in memory, we will first set the allowed_preview_flags_csv if it exists and then put the remaining gflags so that DB servers can accept them.

Test Plan:
Tested manually by setting a preview flag successfully when the flag is passed in allowed_preview_flags_csv and verified that the request is rejected if we do not put it in allowed_preview_flags_csv.
Also, verified that allowed_preview_flags_csv is set first when exists during a non-restart upgrade through logs and Java debugger.

Reviewers: yshchetinin, sanketh

Reviewed By: yshchetinin

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D36687
  • Loading branch information
vipul-yb committed Jul 23, 2024
1 parent 8b23629 commit 4ee3511
Show file tree
Hide file tree
Showing 13 changed files with 399 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import com.yugabyte.yw.commissioner.BaseTaskDependencies;
import com.yugabyte.yw.commissioner.tasks.UniverseTaskBase.ServerType;
import com.yugabyte.yw.commissioner.tasks.params.ServerSubTaskParams;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Supplier;
Expand Down Expand Up @@ -89,18 +90,25 @@ public void run() {
}
YBClient client = getClient();
try {
// allowed_preview_flags_csv should be set first in order to set the preview flags.
if (gflags.containsKey(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV)) {
log.info(
"Setting Allowed Preview Flags for {} on node {}",
taskParams().serverType,
taskParams().nodeName);
setFlag(
client,
GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV,
gflags.get(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV),
hp);
gflags.remove(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV);
log.info(
"Setting remaining flags for {} on node {}",
taskParams().serverType,
taskParams().nodeName);
}
for (Entry<String, String> gflag : gflags.entrySet()) {
boolean setSuccess =
client.setFlag(hp, gflag.getKey(), gflag.getValue(), taskParams().force);
if (!setSuccess) {
throw new RuntimeException(
"Could not set gflag "
+ gflag
+ " for "
+ taskParams().serverType
+ " on node "
+ taskParams().nodeName);
}
setFlag(client, gflag.getKey(), gflag.getValue(), hp);
}
} catch (Exception e) {
log.error("{} hit error : {}", getName(), e.getMessage());
Expand All @@ -109,4 +117,18 @@ public void run() {
closeClient(client);
}
}

private void setFlag(YBClient client, String gflag, String value, HostAndPort hp)
throws Exception {
boolean setSuccess = client.setFlag(hp, gflag, value, taskParams().force);
if (!setSuccess) {
throw new RuntimeException(
"Could not set gflag "
+ gflag
+ " for "
+ taskParams().serverType
+ " on node "
+ taskParams().nodeName);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ public class GFlagsUtil {
"notify_peer_of_removal_from_cluster";
public static final String MASTER_JOIN_EXISTING_UNIVERSE = "master_join_existing_universe";

public static final String ALLOWED_PREVIEW_FLAGS_CSV = "allowed_preview_flags_csv";

private static final Pattern LOG_LINE_PREFIX_PATTERN =
Pattern.compile("^\"?\\s*log_line_prefix\\s*=\\s*'?([^']+)'?\\s*\"?$");
private static final String DEFAULT_LOG_LINE_PREFIX = "%m [%p] ";
Expand Down Expand Up @@ -1714,6 +1716,99 @@ public static String checkForbiddenToOverride(
return null;
}

/**
* Checks the preview flags in the user-provided gflags map against the allowed preview flags.
*
* @param userGFlags The map of user-provided gflags.
* @param ybSoftwareVersion The version of the YB software.
* @param serverType The type of server.
* @param gFlagsValidation The gflags validation object.
* @return A string indicating any issues with the preview flags, or null if there are no issues.
* @throws IOException If an I/O error occurs.
*/
public static String checkPreviewGFlags(
Map<String, String> userGFlags,
String ybSoftwareVersion,
ServerType serverType,
GFlagsValidation gFlagsValidation)
throws IOException {
if (MapUtils.isEmpty(userGFlags)) {
return null;
}
List<String> previewGFlags = new ArrayList<>();
for (String key : userGFlags.keySet()) {
Optional<GFlagDetails> gflagDetails =
gFlagsValidation.getGFlagDetails(ybSoftwareVersion, serverType.name(), key);
if (gflagDetails.isPresent()
&& gflagDetails.get().tags != null
&& gflagDetails.get().tags.contains("preview")) {
previewGFlags.add(key);
}
}
if (previewGFlags.size() == 0) {
return null;
}
String allowedPreviewFlags = userGFlags.get(ALLOWED_PREVIEW_FLAGS_CSV);
if (StringUtils.isEmpty(allowedPreviewFlags)) {
return String.format(
"Universe is equipped with preview flags but %s is missing. ", ALLOWED_PREVIEW_FLAGS_CSV);
}
List<String> allowedPreviewFlagsList =
(Arrays.asList(allowedPreviewFlags.split(",")))
.stream().map(String::trim).collect(Collectors.toList());
for (String previewFlag : previewGFlags) {
if (!allowedPreviewFlagsList.contains(previewFlag)) {
return String.format(
"Universe is equipped with preview flags %s but it is not set in %s : %s ",
previewFlag, ALLOWED_PREVIEW_FLAGS_CSV, allowedPreviewFlags);
}
}
return null;
}

/**
* Checks the preview GFlags on specific GFlags.
*
* @param specificGFlags The specific GFlags to check.
* @param gFlagsValidation The GFlags validation object.
* @param ybSoftwareVersion The YB software version.
* @return An error message if any of the preview GFlags are invalid, otherwise null.
* @throws IOException If an I/O error occurs.
*/
public static String checkPreviewGFlagsOnSpecificGFlags(
SpecificGFlags specificGFlags, GFlagsValidation gFlagsValidation, String ybSoftwareVersion)
throws IOException {
if (specificGFlags == null) {
return null;
}
if (specificGFlags.hasPerAZOverrides()) {
for (UUID azUUID : specificGFlags.getPerAZ().keySet()) {
for (ServerType serverType : Arrays.asList(ServerType.MASTER, ServerType.TSERVER)) {
Map<String, String> perAZGFlags = specificGFlags.getGFlags(azUUID, serverType);
String errMsg =
checkPreviewGFlags(perAZGFlags, ybSoftwareVersion, serverType, gFlagsValidation);
if (errMsg != null) {
return errMsg;
}
}
}
} else {
if (specificGFlags.getPerProcessFlags() != null
&& specificGFlags.getPerProcessFlags().value != null) {
for (ServerType serverType : Arrays.asList(ServerType.MASTER, ServerType.TSERVER)) {
Map<String, String> perProcessGFlags =
specificGFlags.getPerProcessFlags().value.get(serverType);
String errMsg =
checkPreviewGFlags(perProcessGFlags, ybSoftwareVersion, serverType, gFlagsValidation);
if (errMsg != null) {
return errMsg;
}
}
}
}
return null;
}

public static void removeGFlag(
UniverseDefinitionTaskParams.UserIntent userIntent,
String gflagKey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,12 @@ public List<GFlagDetails> extractGFlags(String version, String serverType, boole
}
}

public Optional<GFlagDetails> getGFlagDetails(String version, String serverType, String gflagName)
throws IOException {
List<GFlagDetails> gflagsList = extractGFlags(version, serverType, false);
return gflagsList.stream().filter(flag -> flag.name.equals(gflagName)).findFirst();
}

public List<GFlagGroup> extractGFlagGroups(String version) throws IOException {
InputStream flagStream = null;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,18 @@
import com.yugabyte.yw.common.PlatformServiceException;
import com.yugabyte.yw.common.gflags.GFlagDetails;
import com.yugabyte.yw.common.gflags.GFlagGroup;
import com.yugabyte.yw.common.gflags.GFlagsUtil;
import com.yugabyte.yw.common.gflags.GFlagsValidation;
import com.yugabyte.yw.forms.GFlagsValidationFormData;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagValidationDetails;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagsValidationRequest;
import com.yugabyte.yw.forms.GFlagsValidationFormData.GFlagsValidationResponse;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.regex.Matcher;
Expand Down Expand Up @@ -100,12 +103,39 @@ public List<GFlagsValidationResponse> validateGFlags(
gflagsValidation.extractGFlags(version, ServerType.TSERVER.toString(), false).stream()
.collect(Collectors.toMap(gflag -> gflag.name, Function.identity()));

List<String> allowedPreviewMasterFlags = new ArrayList<>();
List<String> allowedPreviewTServerFlags = new ArrayList<>();
if (gflags.gflagsList.size() > 0) {
Optional<GFlagsValidationRequest> allowedPreviewFlagValidationRequest =
gflags.gflagsList.stream()
.filter(flag -> flag.name.equals(GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV))
.findFirst();
if (allowedPreviewFlagValidationRequest.isPresent()) {
if (allowedPreviewFlagValidationRequest.get().masterValue != null) {
allowedPreviewMasterFlags =
ImmutableList.copyOf(
Arrays.stream(allowedPreviewFlagValidationRequest.get().masterValue.split(","))
.map(String::trim)
.collect(Collectors.toList()));
}
if (allowedPreviewFlagValidationRequest.get().tserverValue != null) {
allowedPreviewTServerFlags =
ImmutableList.copyOf(
Arrays.stream(allowedPreviewFlagValidationRequest.get().tserverValue.split(","))
.map(String::trim)
.collect(Collectors.toList()));
}
}
}

List<GFlagsValidationResponse> validationResponseArrayList = new ArrayList<>();
for (GFlagsValidationRequest gflag : gflags.gflagsList) {
GFlagsValidationResponse validationResponse = new GFlagsValidationResponse();
validationResponse.name = gflag.name;
validationResponse.masterResponse = checkGflags(gflag, ServerType.MASTER, masterGflagsMap);
validationResponse.tserverResponse = checkGflags(gflag, ServerType.TSERVER, tserverGflagsMap);
validationResponse.masterResponse =
checkGflags(gflag, ServerType.MASTER, masterGflagsMap, allowedPreviewMasterFlags);
validationResponse.tserverResponse =
checkGflags(gflag, ServerType.TSERVER, tserverGflagsMap, allowedPreviewTServerFlags);
validationResponseArrayList.add(validationResponse);
}
return validationResponseArrayList;
Expand Down Expand Up @@ -143,7 +173,10 @@ public List<GFlagGroup> getGFlagGroups(String version, String gflagGroup) throws
}

private GFlagValidationDetails checkGflags(
GFlagsValidationRequest gflag, ServerType serverType, Map<String, GFlagDetails> gflags) {
GFlagsValidationRequest gflag,
ServerType serverType,
Map<String, GFlagDetails> gflags,
List<String> allowedPreviewFlags) {
GFlagValidationDetails validationDetails = new GFlagValidationDetails();
GFlagDetails gflagDetails = gflags.get(gflag.name);
if (gflagDetails != null) {
Expand All @@ -159,6 +192,14 @@ private GFlagValidationDetails checkGflags(
validationDetails.error = "Given value is not valid";
}
}
if (gflagDetails.tags != null && gflagDetails.tags.contains("preview")) {
if (!allowedPreviewFlags.contains(gflag.name)) {
validationDetails.error =
String.format(
"Given flag '%s' is not allowed to be set unless it is added in '%s' flag",
gflag.name, GFlagsUtil.ALLOWED_PREVIEW_FLAGS_CSV);
}
}
}
return validationDetails;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
import com.yugabyte.yw.models.helpers.PlacementInfo;
import com.yugabyte.yw.models.helpers.TaskType;
import io.ebean.DB;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -656,6 +657,18 @@ public UniverseResp createUniverse(Customer customer, UniverseDefinitionTaskPara
GFlagsUtil.getBaseGFlags(UniverseTaskBase.ServerType.MASTER, c, taskParams.clusters);
c.userIntent.tserverGFlags =
GFlagsUtil.getBaseGFlags(UniverseTaskBase.ServerType.TSERVER, c, taskParams.clusters);
try {
String errMsg =
GFlagsUtil.checkPreviewGFlagsOnSpecificGFlags(
c.userIntent.specificGFlags, gFlagsValidation, c.userIntent.ybSoftwareVersion);
if (errMsg != null) {
throw new PlatformServiceException(BAD_REQUEST, errMsg);
}
} catch (IOException e) {
LOG.error("Error while checking preview flags on the cluster: {}", c.uuid, e);
throw new PlatformServiceException(
INTERNAL_SERVER_ERROR, "Error while checking preview flags on cluster: " + c.uuid);
}
} else {
if (c.clusterType == ClusterType.ASYNC) {
c.userIntent.specificGFlags = SpecificGFlags.constructInherited();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ public void verifyParams(Universe universe, boolean isFirstTry) {
if (!verifyGFlagsHasChanges(universe) && isFirstTry) {
throw new PlatformServiceException(Http.Status.BAD_REQUEST, SPECIFIC_GFLAGS_NO_CHANGES_ERROR);
}

verifyPreviewGFlagsSettings(universe);
}

public void checkXClusterAutoFlags(
Expand Down
Loading

0 comments on commit 4ee3511

Please sign in to comment.