From a14b2a5f5999b54231cb59a2db9315df20e73659 Mon Sep 17 00:00:00 2001 From: shivzone Date: Tue, 23 Jan 2018 16:29:41 -0800 Subject: [PATCH 1/6] HAWQ-1581. Separate PXF system parameters from user configurable visible parameters --- .../hawq/pxf/api/utilities/InputData.java | 9 +- .../hawq/pxf/api/utilities/ProfilesConf.java | 2 +- .../pxf/api/utilities/ProfilesConfTest.java | 14 +- .../pxf/plugins/hive/HiveDataFragmenter.java | 2 +- .../pxf/plugins/hive/HiveMetadataFetcher.java | 2 +- .../hawq/pxf/plugins/jdbc/JdbcPlugin.java | 4 +- .../apache/hawq/pxf/plugins/json/PxfUnit.java | 16 +- .../pxf/service/utilities/ProtocolData.java | 30 ++-- pxf/pxf-service/src/scripts/pxf-service | 143 ++++++++++-------- .../pxf/service/BridgeOutputBuilderTest.java | 14 +- .../service/utilities/ProtocolDataTest.java | 66 ++++---- 11 files changed, 166 insertions(+), 136 deletions(-) diff --git a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/InputData.java b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/InputData.java index 7bf57ed05c..de9d8e9bfe 100644 --- a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/InputData.java +++ b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/InputData.java @@ -23,15 +23,18 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import java.util.*; +import java.util.Map; +import java.util.ArrayList; + /** * Common configuration available to all PXF plugins. Represents input data - * coming from client applications, such as Hawq. + * coming from client applications, such as Hawq or GPDB. */ public class InputData { public static final String DELIMITER_KEY = "DELIMITER"; + public static final String USER_PROP_PREFIX = "X-GP-OPTIONS-"; public static final int INVALID_SPLIT_IDX = -1; private static final Log LOG = LogFactory.getLog(InputData.class); @@ -125,7 +128,7 @@ public InputData(InputData copy) { * @return property value as a String */ public String getUserProperty(String userProp) { - return requestParametersMap.get("X-GP-" + userProp.toUpperCase()); + return requestParametersMap.get(USER_PROP_PREFIX + userProp.toUpperCase()); } /** diff --git a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ProfilesConf.java b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ProfilesConf.java index 2c20ab7c9d..e3e8b71b8d 100644 --- a/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ProfilesConf.java +++ b/pxf/pxf-api/src/main/java/org/apache/hawq/pxf/api/utilities/ProfilesConf.java @@ -129,7 +129,7 @@ private Map getProfilePluginMap(Configuration profileSubset) { for (String plugin : plugins) { String pluginValue = profileSubset.getString(plugin); if (!StringUtils.isEmpty(StringUtils.trim(pluginValue))) { - pluginsMap.put("X-GP-" + plugin.toUpperCase(), pluginValue); + pluginsMap.put(InputData.USER_PROP_PREFIX + plugin.toUpperCase(), pluginValue); } } return pluginsMap; diff --git a/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java b/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java index 0631cb2b05..295912109e 100644 --- a/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java +++ b/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java @@ -96,16 +96,18 @@ public void definedProfile() throws Exception { optionalFile.toURI().toURL()); Map hbaseProfile = ProfilesConf.getProfilePluginsMap("HBase"); + System.out.println(hbaseProfile); + System.out.println(hbaseProfile.get("X-GP-OPTIONS-PLUGIN1")); assertEquals(2, hbaseProfile.keySet().size()); - assertEquals(hbaseProfile.get("X-GP-PLUGIN1"), "X"); - assertEquals(hbaseProfile.get("X-GP-PLUGIN2"), "XX"); + assertEquals(hbaseProfile.get("X-GP-OPTIONS-PLUGIN1"), "X"); + assertEquals(hbaseProfile.get("X-GP-OPTIONS-PLUGIN2"), "XX"); Map hiveProfile = ProfilesConf.getProfilePluginsMap("hIVe");// case // insensitive // profile // name assertEquals(1, hiveProfile.keySet().size()); - assertEquals(hiveProfile.get("X-GP-PLUGIN1"), "Y"); + assertEquals(hiveProfile.get("X-GP-OPTIONS-PLUGIN1"), "Y"); Mockito.verify(LOG).info("PXF profiles loaded: [HBase, Hive]"); } @@ -170,8 +172,8 @@ public void overrideProfile() throws Exception { optionalFile.toURI().toURL()); Map profile = ProfilesConf.getProfilePluginsMap("HBase"); assertEquals(2, profile.keySet().size()); - assertEquals(profile.get("X-GP-PLUGIN1"), "Y"); - assertEquals(profile.get("X-GP-PLUGIN2"), "YY"); + assertEquals(profile.get("X-GP-OPTIONS-PLUGIN1"), "Y"); + assertEquals(profile.get("X-GP-OPTIONS-PLUGIN2"), "YY"); } } @@ -242,7 +244,7 @@ public void missingOptionalProfileFile() throws Exception { mandatoryFile.toURI().toURL()); when(classLoader.getResource("pxf-profiles.xml")).thenReturn(null); Map hbaseProfile = ProfilesConf.getProfilePluginsMap("HBase"); - assertEquals("Y", hbaseProfile.get("X-GP-PLUGIN1")); + assertEquals("Y", hbaseProfile.get("X-GP-OPTIONS-PLUGIN1")); Mockito.verify(LOG).warn("pxf-profiles.xml not found in the classpath"); } } diff --git a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java index 76b83e6729..406630cfe0 100644 --- a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java +++ b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveDataFragmenter.java @@ -299,7 +299,7 @@ private void fetchMetaData(HiveTablePartition tablePartition, boolean hasComplex } String fragmenterForProfile = null; if (profile != null) { - fragmenterForProfile = ProfilesConf.getProfilePluginsMap(profile).get("X-GP-FRAGMENTER"); + fragmenterForProfile = ProfilesConf.getProfilePluginsMap(profile).get("X-GP-OPTIONS-FRAGMENTER"); } else { fragmenterForProfile = inputData.getFragmenter(); } diff --git a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveMetadataFetcher.java b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveMetadataFetcher.java index dc76289ddc..2a27b88956 100644 --- a/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveMetadataFetcher.java +++ b/pxf/pxf-hive/src/main/java/org/apache/hawq/pxf/plugins/hive/HiveMetadataFetcher.java @@ -137,7 +137,7 @@ private OutputFormat getOutputFormat(String inputFormat, boolean hasComplexTypes OutputFormat outputFormat = null; InputFormat fformat = HiveDataFragmenter.makeInputFormat(inputFormat, jobConf); String profile = ProfileFactory.get(fformat, hasComplexTypes); - String outputFormatClassName = ProfilesConf.getProfilePluginsMap(profile).get("X-GP-OUTPUTFORMAT"); + String outputFormatClassName = ProfilesConf.getProfilePluginsMap(profile).get("X-GP-OPTIONS-OUTPUTFORMAT"); outputFormat = OutputFormat.getOutputFormat(outputFormatClassName); return outputFormat; } diff --git a/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java b/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java index 2e03264b72..c183bbb51f 100644 --- a/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java +++ b/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java @@ -58,8 +58,8 @@ public JdbcPlugin(InputData input) throws UserDataException { super(input); jdbcDriver = input.getUserProperty("JDBC_DRIVER"); dbUrl = input.getUserProperty("DB_URL"); - user = input.getUserProperty("USER"); - pass = input.getUserProperty("PASS"); + user = input.getUserProperty("DB_USER"); + pass = input.getUserProperty("DB_PASS"); String strBatch = input.getUserProperty("BATCH_SIZE"); if (strBatch != null) { batchSize = Integer.parseInt(strBatch); diff --git a/pxf/pxf-json/src/test/java/org/apache/hawq/pxf/plugins/json/PxfUnit.java b/pxf/pxf-json/src/test/java/org/apache/hawq/pxf/plugins/json/PxfUnit.java index 5669882e7c..c9ba4d07cf 100644 --- a/pxf/pxf-json/src/test/java/org/apache/hawq/pxf/plugins/json/PxfUnit.java +++ b/pxf/pxf-json/src/test/java/org/apache/hawq/pxf/plugins/json/PxfUnit.java @@ -246,7 +246,7 @@ public Class getWriteResolverClass() { } /** - * Get any extra parameters that are meant to be specified for the "pxf" protocol. Note that "X-GP-" is prepended to + * Get any extra parameters that are meant to be specified for the "pxf" protocol. Note that "X-GP-OPTIONS-" is prepended to * each parameter name. * * @return Any extra parameters or null if none. @@ -301,12 +301,12 @@ protected InputData getInputDataForWritableTable(Path input) { paramsMap.put("X-GP-ATTR-TYPECODE" + i, Integer.toString(params.get(i).second.getOID())); } - paramsMap.put("X-GP-ACCESSOR", getWriteAccessorClass().getName()); - paramsMap.put("X-GP-RESOLVER", getWriteResolverClass().getName()); + paramsMap.put("X-GP-OPTIONS-ACCESSOR", getWriteAccessorClass().getName()); + paramsMap.put("X-GP-OPTIONS-RESOLVER", getWriteResolverClass().getName()); if (getExtraParams() != null) { for (Pair param : getExtraParams()) { - paramsMap.put("X-GP-" + param.first, param.second); + paramsMap.put("X-GP-OPTIONS-" + param.first, param.second); } } @@ -342,7 +342,6 @@ protected void setup(Path input) throws Exception { paramsMap.put("X-GP-SEGMENT-ID", "1"); paramsMap.put("X-GP-HAS-FILTER", "0"); paramsMap.put("X-GP-SEGMENT-COUNT", "1"); - paramsMap.put("X-GP-FRAGMENTER", getFragmenterClass().getName()); paramsMap.put("X-GP-FORMAT", "GPDBWritable"); paramsMap.put("X-GP-URL-HOST", "localhost"); paramsMap.put("X-GP-URL-PORT", "50070"); @@ -358,12 +357,13 @@ protected void setup(Path input) throws Exception { } // HDFSMetaData properties - paramsMap.put("X-GP-ACCESSOR", getReadAccessorClass().getName()); - paramsMap.put("X-GP-RESOLVER", getReadResolverClass().getName()); + paramsMap.put("X-GP-OPTIONS-FRAGMENTER", getFragmenterClass().getName()); + paramsMap.put("X-GP-OPTIONS-ACCESSOR", getReadAccessorClass().getName()); + paramsMap.put("X-GP-OPTIONS-RESOLVER", getReadResolverClass().getName()); if (getExtraParams() != null) { for (Pair param : getExtraParams()) { - paramsMap.put("X-GP-" + param.first, param.second); + paramsMap.put("X-GP-OPTIONS-" + param.first, param.second); } } diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java index 65cdf16bba..0c9e531b05 100644 --- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java +++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java @@ -57,8 +57,9 @@ public class ProtocolData extends InputData { protected float statsSampleRatio; /** - * Constructs a ProtocolData. Parses X-GP-* configuration variables. - * + * Constructs a ProtocolData. + * Parses X-GP-* system configuration variables and + * X-GP-OPTIONS-* user configuration variables * @param paramsMap contains all query-specific parameters from Hawq */ public ProtocolData(Map paramsMap) { @@ -82,19 +83,26 @@ public ProtocolData(Map paramsMap) { parseTupleDescription(); /* - * accessor - will throw exception from getPropery() if outputFormat is + * accessor - will throw exception if outputFormat is * BINARY and the user did not supply accessor=... or profile=... - * resolver - will throw exception from getPropery() if outputFormat is + * resolver - will throw exception if outputFormat is * BINARY and the user did not supply resolver=... or profile=... */ - profile = getOptionalProperty("PROFILE"); + profile = getUserProperty("PROFILE"); if (profile != null) { setProfilePlugins(); } - accessor = getProperty("ACCESSOR"); - resolver = getProperty("RESOLVER"); - fragmenter = getOptionalProperty("FRAGMENTER"); - metadata = getOptionalProperty("METADATA"); + accessor = getUserProperty("ACCESSOR"); + if(accessor == null) { + protocolViolation(accessor); + } + resolver = getUserProperty("RESOLVER"); + if(resolver == null) { + protocolViolation(resolver); + } + + fragmenter = getUserProperty("FRAGMENTER"); + metadata = getUserProperty("METADATA"); dataSource = getProperty("DATA-DIR"); /* Kerberos token information */ @@ -354,7 +362,7 @@ public float getStatsSampleRatio() { private void parseThreadSafe() { threadSafe = true; - String threadSafeStr = getOptionalProperty("THREAD-SAFE"); + String threadSafeStr = getUserProperty("THREAD-SAFE"); if (threadSafeStr != null) { threadSafe = parseBooleanValue(threadSafeStr); } @@ -491,7 +499,7 @@ private void parseRemoteCredentials() { private void parseStatsParameters() { - String maxFrags = getOptionalProperty("STATS-MAX-FRAGMENTS"); + String maxFrags = getUserProperty("STATS-MAX-FRAGMENTS"); if (!StringUtils.isEmpty(maxFrags)) { statsMaxFragments = Integer.parseInt(maxFrags); if (statsMaxFragments <= 0) { diff --git a/pxf/pxf-service/src/scripts/pxf-service b/pxf/pxf-service/src/scripts/pxf-service index d06127978e..bf2865a242 100644 --- a/pxf/pxf-service/src/scripts/pxf-service +++ b/pxf/pxf-service/src/scripts/pxf-service @@ -21,25 +21,25 @@ # -if [ -z ${PXF_HOME} ]; then +if [ -z $PXF_HOME ]; then parent_script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" - env_script=${parent_script_dir}/conf/pxf-env.sh + env_script=$parent_script_dir/conf/pxf-env.sh else - env_script=${PXF_HOME}/conf/pxf-env.sh + env_script=$PXF_HOME/conf/pxf-env.sh fi # load pxf-env.sh script -if [ ! -f ${env_script} ]; then +if [ ! -f $env_script ]; then echo WARNING: failed to find $env_script else - source ${env_script} + source $env_script fi pxf_user=${PXF_USER} -instance_port=${PXF_PORT:-@pxfPortNum@} +instance_port=${PXF_PORT:-51200} instance_name=pxf-service -if [ -z ${PXF_HOME} ]; then +if [ -z $PXF_HOME ]; then # RPM based setup pxf_root=/usr/lib/pxf tomcat_root=/opt/apache-tomcat @@ -48,11 +48,11 @@ if [ -z ${PXF_HOME} ]; then instance_owner=$pxf_user:$pxf_user else # OSS/Source code based setup - pxf_root=${PXF_HOME}/lib - tomcat_root=${PXF_HOME}/apache-tomcat - tomcat_templates=${PXF_HOME}/tomcat-templates - instance_root=${PXF_HOME} - instance_owner=${pxf_user} + pxf_root=$PXF_HOME/lib + tomcat_root=$PXF_HOME/apache-tomcat + tomcat_templates=$PXF_HOME/tomcat-templates + instance_root=$PXF_HOME + instance_owner=$pxf_user fi curl=`which curl` @@ -70,25 +70,25 @@ function fail() # function createInstance() { - mkdir -p ${instance_root} - mkdir -p ${instance_root}/${instance_name} - cp -r ${tomcat_root}/* ${instance_root}/${instance_name}/. + mkdir -p $instance_root + mkdir -p $instance_root/$instance_name + cp -r $tomcat_root/* $instance_root/$instance_name/. if [ $? -gt 0 ]; then echo ERROR: instance creation failed return 1 fi - if [ ! -z ${pxf_user} ]; then - chown -R ${instance_owner} ${instance_root} + if [ ! -z $pxf_user ]; then + chown -R $instance_owner $instance_root fi - chmod 700 ${instance_root}/${instance_name} + chmod 700 $instance_root/$instance_name # copy configuration files into instance - cp ${tomcat_templates}/bin/setenv.sh ${instance_root}/${instance_name}/bin/setenv.sh - cp ${tomcat_templates}/conf/catalina.properties ${instance_root}/${instance_name}/conf/. - cp ${tomcat_templates}/conf/server.xml ${instance_root}/${instance_name}/conf/. - cp ${tomcat_templates}/conf/web.xml ${instance_root}/${instance_name}/conf/. + cp $tomcat_templates/bin/setenv.sh $instance_root/$instance_name/bin/setenv.sh + cp $tomcat_templates/conf/catalina.properties $instance_root/$instance_name/conf/. + cp $tomcat_templates/conf/server.xml $instance_root/$instance_name/conf/. + cp $tomcat_templates/conf/web.xml $instance_root/$instance_name/conf/. return 0 } @@ -98,8 +98,8 @@ function createInstance() # function deployWebapp() { - cp ${pxf_root}/pxf.war ${instance_root}/${instance_name}/webapps/ || return 1 - cp ${pxf_root}/pxf-service-*[0-9].jar ${instance_root}/${instance_name}/lib/ || return 1 + cp $pxf_root/pxf.war $instance_root/$instance_name/webapps/ || return 1 + cp $pxf_root/pxf-service-*[0-9].jar $instance_root/$instance_name/lib/ || return 1 return 0 } @@ -124,7 +124,7 @@ function waitForTomcat() return 1 fi echo "tomcat not responding, re-trying after ${sleep_time} second (attempt number ${attempts})" - sleep ${sleep_time} + sleep $sleep_time done return 0 @@ -139,11 +139,11 @@ function checkWebapp() waitForTomcat $1 || return 1 echo Checking if PXF webapp is up and running... - curlResponse=$(${curl} -s "http://localhost:${instance_port}/pxf/v0") + curlResponse=$($curl -s "http://localhost:${instance_port}/pxf/v0") expectedResponse="Wrong version v0, supported version is v[0-9]+" - if [[ ${curlResponse} =~ $expectedResponse ]]; then - echo PXF webapp is listening on port ${instance_port} + if [[ $curlResponse =~ $expectedResponse ]]; then + echo PXF webapp is up return 0 fi @@ -159,7 +159,7 @@ function instanceExists() return 1 fi - ${instance_root}/${instance_name}/bin/catalina.sh version > /dev/null 2>&1 + $instance_root/$instance_name/bin/catalina.sh version > /dev/null 2>&1 return $? } @@ -168,7 +168,7 @@ function doInit() { instanceExists if [ $? -eq 0 ]; then - echo WARNING: instance already exists in ${instance_root}, nothing to do... + echo WARNING: instance already exists in $instance_root, nothing to do... return 0 fi determineHadoopDistro @@ -185,74 +185,91 @@ function doInit() # function configureWebapp() { - if [ -z ${PXF_HOME} ]; then + if [ -z $PXF_HOME ]; then # webapp doesn't require patch return 0 fi - pushd ${instance_root}/${instance_name}/webapps > /dev/null || return 1 + pushd $instance_root/$instance_name/webapps || return 1 rm -rf pxf mkdir pxf cd pxf unzip -q ../pxf.war - popd > /dev/null + popd - context_file=${instance_root}/${instance_name}/webapps/pxf/META-INF/context.xml - sed -i -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"${PXF_HOME}\/conf\/pxf-private.classpath\":" \ - -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"${PXF_HOME}\/conf\/pxf-public.classpath\":" ${context_file} + context_file=$instance_root/$instance_name/webapps/pxf/META-INF/context.xml + cat $context_file | \ + sed -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"$PXF_HOME\/conf\/pxf-private.classpath\":" \ + -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"$PXF_HOME\/conf\/pxf-public.classpath\":" > context.xml.tmp + mv context.xml.tmp $context_file - web_file=${instance_root}/${instance_name}/webapps/pxf/WEB-INF/web.xml - sed -i "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" ${web_file} + web_file=$instance_root/$instance_name/webapps/pxf/WEB-INF/web.xml + cat $web_file | \ + sed "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" > web.xml.tmp + mv web.xml.tmp $web_file # set port - catalinaProperties=${instance_root}/${instance_name}/conf/catalina.properties - sed -i "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" ${catalinaProperties} + catalinaProperties=$instance_root/$instance_name/conf/catalina.properties + cat $catalinaProperties | \ + sed "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" \ + > ${catalinaProperties}.tmp + + rm $catalinaProperties + mv ${catalinaProperties}.tmp $catalinaProperties # set container configs - catalinaEnv=${instance_root}/${instance_name}/bin/setenv.sh - sed -i -e "s|JVM_OPTS=.*$|JVM_OPTS=\"${PXF_JVM_OPTS}\"|g" \ - -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=${PXF_LOGDIR} |g" \ - -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=${PXF_RUNDIR}/catalina.pid|g" \ - -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=${PXF_LOGDIR}/catalina.out|g" ${catalinaEnv} + catalinaEnv=$instance_root/$instance_name/bin/setenv.sh + cat $catalinaEnv | \ + sed -e "s|JVM_OPTS=.*$|JVM_OPTS=\"$PXF_JVM_OPTS\"|g" | \ + sed -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=$PXF_LOGDIR |g" | \ + sed -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=$PXF_RUNDIR/catalina.pid|g" | \ + sed -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=$PXF_LOGDIR/catalina.out|g" \ + > ${catalinaEnv}.tmp + rm $catalinaEnv + mv ${catalinaEnv}.tmp $catalinaEnv # set log directories - catalinaLog=${instance_root}/$instance_name/conf/logging.properties - sed -i "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = ${PXF_LOGDIR}|g" ${catalinaLog} + catalinaLog=$instance_root/$instance_name/conf/logging.properties + cat $catalinaLog | \ + sed "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = $PXF_LOGDIR|g" \ + > ${catalinaLog}.tmp + rm $catalinaLog + mv ${catalinaLog}.tmp $catalinaLog } function commandWebapp() { command=$1 - pushd ${instance_root} > /dev/null - if [ ! -z ${pxf_user} ]; then + pushd $instance_root + if [ ! -z $pxf_user ]; then # Run command as a pxf_user - su ${pxf_user} -c "$instance_root/$instance_name/bin/catalina.sh $command" + su $pxf_user -c "$instance_root/$instance_name/bin/catalina.sh $command" else # Run command as a current user - ${instance_root}/${instance_name}/bin/catalina.sh ${command} + $instance_root/$instance_name/bin/catalina.sh $command fi if [ $? -ne 0 ]; then return 1 fi - popd > /dev/null + popd } function createLogsDir() { - mkdir -p ${PXF_LOGDIR} - if [ ! -z ${pxf_user} ]; then - chown -R ${instance_owner} ${PXF_LOGDIR} + mkdir -p $PXF_LOGDIR + if [ ! -z $pxf_user ]; then + chown -R $instance_owner $PXF_LOGDIR fi - chmod 700 ${PXF_LOGDIR} + chmod 700 $PXF_LOGDIR return 0 } function createRunDir() { - mkdir -p ${PXF_RUNDIR} - if [ ! -z ${pxf_user} ]; then - chown -R ${instance_owner} ${PXF_RUNDIR} + mkdir -p $PXF_RUNDIR + if [ ! -z $pxf_user ]; then + chown -R $instance_owner $PXF_RUNDIR fi - chmod 700 ${PXF_RUNDIR} + chmod 700 $PXF_RUNDIR return 0 } @@ -362,8 +379,8 @@ function validateParameters() fi # validate pxf user - if [ ! -z ${pxf_user} ]; then - id ${pxf_user} &> /dev/null; + if [ ! -z $pxf_user ]; then + id $pxf_user &> /dev/null; if [ "$?" -ne "0" ] then echo "ERROR: User $pxf_user doesn't exist. Please set valid user via \$PXF_USER variable" @@ -378,7 +395,7 @@ function validateParameters() fi # validate JAVA_HOME - if [ ! -x ${JAVA_HOME}/bin/java ]; then + if [ ! -x $JAVA_HOME/bin/java ]; then echo ERROR: \$JAVA_HOME is invalid exit 1 fi diff --git a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/BridgeOutputBuilderTest.java b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/BridgeOutputBuilderTest.java index a00910df48..9dbd906a5b 100644 --- a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/BridgeOutputBuilderTest.java +++ b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/BridgeOutputBuilderTest.java @@ -324,8 +324,8 @@ public void convertTextDataToLines() throws Exception { parameters.put("X-GP-ATTRS", "1"); addColumn(parameters, 0, DataType.TEXT, "col0"); // activate sampling code - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "100"); - parameters.put("X-GP-STATS-SAMPLE-RATIO", "1.00"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "100"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "1.00"); BridgeOutputBuilder builder = makeBuilder(parameters); LinkedList outputQueue = builder.makeOutput(fields); @@ -352,8 +352,8 @@ public void convertTextDataToLinesPartial() throws Exception { parameters.put("X-GP-ATTRS", "1"); addColumn(parameters, 0, DataType.TEXT, "col0"); // activate sampling code - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "100"); - parameters.put("X-GP-STATS-SAMPLE-RATIO", "1.00"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "100"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "1.00"); BridgeOutputBuilder builder = makeBuilder(parameters); LinkedList outputQueue = builder.makeOutput(fields); @@ -464,11 +464,11 @@ private BridgeOutputBuilder makeBuilder(Map parameters) parameters.put("X-GP-FORMAT", "TEXT"); parameters.put("X-GP-URL-HOST", "my://bags"); parameters.put("X-GP-URL-PORT", "-8020"); - parameters.put("X-GP-ACCESSOR", "are"); - parameters.put("X-GP-RESOLVER", "packed"); + parameters.put("X-GP-OPTIONS-ACCESSOR", "are"); + parameters.put("X-GP-OPTIONS-RESOLVER", "packed"); parameters.put("X-GP-DATA-DIR", "i'm/ready/to/go"); parameters.put("X-GP-FRAGMENT-METADATA", "U29tZXRoaW5nIGluIHRoZSB3YXk="); - parameters.put("X-GP-I'M-STANDING-HERE", "outside-your-door"); + parameters.put("X-GP-OPTIONS-I'M-STANDING-HERE", "outside-your-door"); ProtocolData protocolData = new ProtocolData(parameters); BridgeOutputBuilder builder = new BridgeOutputBuilder(protocolData); diff --git a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java index 10479c797b..6cbd2f1246 100644 --- a/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java +++ b/pxf/pxf-service/src/test/java/org/apache/hawq/pxf/service/utilities/ProtocolDataTest.java @@ -94,7 +94,7 @@ public void profileWithDuplicateProperty() throws Exception { when(ProfilesConf.getProfilePluginsMap("a profile")).thenReturn( mockedProfiles); - parameters.put("x-gp-profile", "a profile"); + parameters.put("x-gp-options-profile", "a profile"); parameters.put("when you try your best", "and you do succeed"); parameters.put("WHEN you GET what YOU want", "and what you need"); @@ -110,9 +110,9 @@ public void profileWithDuplicateProperty() throws Exception { @Test public void definedProfile() throws Exception { - parameters.put("X-GP-PROFILE", "HIVE"); - parameters.remove("X-GP-ACCESSOR"); - parameters.remove("X-GP-RESOLVER"); + parameters.put("X-GP-OPTIONS-PROFILE", "HIVE"); + parameters.remove("X-GP-OPTIONS-ACCESSOR"); + parameters.remove("X-GP-OPTIONS-RESOLVER"); ProtocolData protocolData = new ProtocolData(parameters); assertEquals(protocolData.getFragmenter(), "org.apache.hawq.pxf.plugins.hive.HiveDataFragmenter"); assertEquals(protocolData.getAccessor(), "org.apache.hawq.pxf.plugins.hive.HiveAccessor"); @@ -121,7 +121,7 @@ public void definedProfile() throws Exception { @Test public void undefinedProfile() throws Exception { - parameters.put("X-GP-PROFILE", "THIS_PROFILE_NEVER_EXISTED!"); + parameters.put("X-GP-OPTIONS-PROFILE", "THIS_PROFILE_NEVER_EXISTED!"); try { new ProtocolData(parameters); fail("Undefined profile should throw ProfileConfException"); @@ -132,29 +132,29 @@ public void undefinedProfile() throws Exception { @Test public void threadSafeTrue() throws Exception { - parameters.put("X-GP-THREAD-SAFE", "TRUE"); + parameters.put("X-GP-OPTIONS-THREAD-SAFE", "TRUE"); ProtocolData protocolData = new ProtocolData(parameters); assertEquals(protocolData.isThreadSafe(), true); - parameters.put("X-GP-THREAD-SAFE", "true"); + parameters.put("X-GP-OPTIONS-THREAD-SAFE", "true"); protocolData = new ProtocolData(parameters); assertEquals(protocolData.isThreadSafe(), true); } @Test public void threadSafeFalse() throws Exception { - parameters.put("X-GP-THREAD-SAFE", "False"); + parameters.put("X-GP-OPTIONS-THREAD-SAFE", "False"); ProtocolData protocolData = new ProtocolData(parameters); assertEquals(protocolData.isThreadSafe(), false); - parameters.put("X-GP-THREAD-SAFE", "falSE"); + parameters.put("X-GP-OPTIONS-THREAD-SAFE", "falSE"); protocolData = new ProtocolData(parameters); assertEquals(protocolData.isThreadSafe(), false); } @Test public void threadSafeMaybe() throws Exception { - parameters.put("X-GP-THREAD-SAFE", "maybe"); + parameters.put("X-GP-OPTIONS-THREAD-SAFE", "maybe"); try { new ProtocolData(parameters); fail("illegal THREAD-SAFE value should throw IllegalArgumentException"); @@ -166,7 +166,7 @@ public void threadSafeMaybe() throws Exception { @Test public void threadSafeDefault() throws Exception { - parameters.remove("X-GP-THREAD-SAFE"); + parameters.remove("X-GP-OPTIONS-THREAD-SAFE"); ProtocolData protocolData = new ProtocolData(parameters); assertEquals(protocolData.isThreadSafe(), true); } @@ -232,8 +232,8 @@ public void noStatsParams() { @Test public void statsParams() { - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "10101"); - parameters.put("X-GP-STATS-SAMPLE-RATIO", "0.039"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "10101"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "0.039"); ProtocolData protData = new ProtocolData(parameters); @@ -243,21 +243,21 @@ public void statsParams() { @Test public void statsMissingParams() { - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "13"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "13"); try { new ProtocolData(parameters); - fail("missing X-GP-STATS-SAMPLE-RATIO parameter"); + fail("missing X-GP-OPTIONS-STATS-SAMPLE-RATIO parameter"); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), "Missing parameter: STATS-SAMPLE-RATIO and STATS-MAX-FRAGMENTS must be set together"); } - parameters.remove("X-GP-STATS-MAX-FRAGMENTS"); - parameters.put("X-GP-STATS-SAMPLE-RATIO", "1"); + parameters.remove("X-GP-OPTIONS-STATS-MAX-FRAGMENTS"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "1"); try { new ProtocolData(parameters); - fail("missing X-GP-STATS-MAX-FRAGMENTS parameter"); + fail("missing X-GP-OPTIONS-STATS-MAX-FRAGMENTS parameter"); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -267,11 +267,11 @@ public void statsMissingParams() { @Test public void statsSampleRatioNegative() { - parameters.put("X-GP-STATS-SAMPLE-RATIO", "101"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "101"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-SAMPLE-RATIO value"); + fail("wrong X-GP-OPTIONS-STATS-SAMPLE-RATIO value"); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -279,10 +279,10 @@ public void statsSampleRatioNegative() { + "STATS-SAMPLE-RATIO must be a value between 0.0001 and 1.0"); } - parameters.put("X-GP-STATS-SAMPLE-RATIO", "0"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "0"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-SAMPLE-RATIO value"); + fail("wrong X-GP-OPTIONS-STATS-SAMPLE-RATIO value"); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -290,10 +290,10 @@ public void statsSampleRatioNegative() { + "STATS-SAMPLE-RATIO must be a value between 0.0001 and 1.0"); } - parameters.put("X-GP-STATS-SAMPLE-RATIO", "0.00005"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "0.00005"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-SAMPLE-RATIO value"); + fail("wrong X-GP-OPTIONS-STATS-SAMPLE-RATIO value"); } catch (IllegalArgumentException e) { assertEquals( e.getMessage(), @@ -301,10 +301,10 @@ public void statsSampleRatioNegative() { + "STATS-SAMPLE-RATIO must be a value between 0.0001 and 1.0"); } - parameters.put("X-GP-STATS-SAMPLE-RATIO", "a"); + parameters.put("X-GP-OPTIONS-STATS-SAMPLE-RATIO", "a"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-SAMPLE-RATIO value"); + fail("wrong X-GP-OPTIONS-STATS-SAMPLE-RATIO value"); } catch (NumberFormatException e) { assertEquals(e.getMessage(), "For input string: \"a\""); } @@ -312,20 +312,20 @@ public void statsSampleRatioNegative() { @Test public void statsMaxFragmentsNegative() { - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "10.101"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "10.101"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-MAX-FRAGMENTS value"); + fail("wrong X-GP-OPTIONS-STATS-MAX-FRAGMENTS value"); } catch (NumberFormatException e) { assertEquals(e.getMessage(), "For input string: \"10.101\""); } - parameters.put("X-GP-STATS-MAX-FRAGMENTS", "0"); + parameters.put("X-GP-OPTIONS-STATS-MAX-FRAGMENTS", "0"); try { new ProtocolData(parameters); - fail("wrong X-GP-STATS-MAX-FRAGMENTS value"); + fail("wrong X-GP-OPTIONS-STATS-MAX-FRAGMENTS value"); } catch (IllegalArgumentException e) { assertEquals(e.getMessage(), "Wrong value '0'. " + "STATS-MAX-FRAGMENTS must be a positive integer"); @@ -425,11 +425,11 @@ public void setUp() { parameters.put("X-GP-URL-HOST", "my://bags"); parameters.put("X-GP-URL-PORT", "-8020"); parameters.put("X-GP-ATTRS", "-1"); - parameters.put("X-GP-ACCESSOR", "are"); - parameters.put("X-GP-RESOLVER", "packed"); + parameters.put("X-GP-OPTIONS-ACCESSOR", "are"); + parameters.put("X-GP-OPTIONS-RESOLVER", "packed"); parameters.put("X-GP-DATA-DIR", "i'm/ready/to/go"); parameters.put("X-GP-FRAGMENT-METADATA", "U29tZXRoaW5nIGluIHRoZSB3YXk="); - parameters.put("X-GP-I'M-STANDING-HERE", "outside-your-door"); + parameters.put("X-GP-OPTIONS-I'M-STANDING-HERE", "outside-your-door"); PowerMockito.mockStatic(UserGroupInformation.class); } From 869ace8b80bc1b8d133c17d2b05b10ba2b861f19 Mon Sep 17 00:00:00 2001 From: shivzone Date: Wed, 24 Jan 2018 13:26:41 -0800 Subject: [PATCH 2/6] Code Review fixes --- .../pxf/api/utilities/ProfilesConfTest.java | 2 - .../hawq/pxf/plugins/jdbc/JdbcPlugin.java | 4 +- pxf/pxf-service/src/scripts/pxf-service | 143 ++++++++---------- 3 files changed, 65 insertions(+), 84 deletions(-) diff --git a/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java b/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java index 295912109e..32093ccd58 100644 --- a/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java +++ b/pxf/pxf-api/src/test/java/org/apache/hawq/pxf/api/utilities/ProfilesConfTest.java @@ -96,8 +96,6 @@ public void definedProfile() throws Exception { optionalFile.toURI().toURL()); Map hbaseProfile = ProfilesConf.getProfilePluginsMap("HBase"); - System.out.println(hbaseProfile); - System.out.println(hbaseProfile.get("X-GP-OPTIONS-PLUGIN1")); assertEquals(2, hbaseProfile.keySet().size()); assertEquals(hbaseProfile.get("X-GP-OPTIONS-PLUGIN1"), "X"); assertEquals(hbaseProfile.get("X-GP-OPTIONS-PLUGIN2"), "XX"); diff --git a/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java b/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java index c183bbb51f..2e03264b72 100644 --- a/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java +++ b/pxf/pxf-jdbc/src/main/java/org/apache/hawq/pxf/plugins/jdbc/JdbcPlugin.java @@ -58,8 +58,8 @@ public JdbcPlugin(InputData input) throws UserDataException { super(input); jdbcDriver = input.getUserProperty("JDBC_DRIVER"); dbUrl = input.getUserProperty("DB_URL"); - user = input.getUserProperty("DB_USER"); - pass = input.getUserProperty("DB_PASS"); + user = input.getUserProperty("USER"); + pass = input.getUserProperty("PASS"); String strBatch = input.getUserProperty("BATCH_SIZE"); if (strBatch != null) { batchSize = Integer.parseInt(strBatch); diff --git a/pxf/pxf-service/src/scripts/pxf-service b/pxf/pxf-service/src/scripts/pxf-service index bf2865a242..d06127978e 100644 --- a/pxf/pxf-service/src/scripts/pxf-service +++ b/pxf/pxf-service/src/scripts/pxf-service @@ -21,25 +21,25 @@ # -if [ -z $PXF_HOME ]; then +if [ -z ${PXF_HOME} ]; then parent_script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" - env_script=$parent_script_dir/conf/pxf-env.sh + env_script=${parent_script_dir}/conf/pxf-env.sh else - env_script=$PXF_HOME/conf/pxf-env.sh + env_script=${PXF_HOME}/conf/pxf-env.sh fi # load pxf-env.sh script -if [ ! -f $env_script ]; then +if [ ! -f ${env_script} ]; then echo WARNING: failed to find $env_script else - source $env_script + source ${env_script} fi pxf_user=${PXF_USER} -instance_port=${PXF_PORT:-51200} +instance_port=${PXF_PORT:-@pxfPortNum@} instance_name=pxf-service -if [ -z $PXF_HOME ]; then +if [ -z ${PXF_HOME} ]; then # RPM based setup pxf_root=/usr/lib/pxf tomcat_root=/opt/apache-tomcat @@ -48,11 +48,11 @@ if [ -z $PXF_HOME ]; then instance_owner=$pxf_user:$pxf_user else # OSS/Source code based setup - pxf_root=$PXF_HOME/lib - tomcat_root=$PXF_HOME/apache-tomcat - tomcat_templates=$PXF_HOME/tomcat-templates - instance_root=$PXF_HOME - instance_owner=$pxf_user + pxf_root=${PXF_HOME}/lib + tomcat_root=${PXF_HOME}/apache-tomcat + tomcat_templates=${PXF_HOME}/tomcat-templates + instance_root=${PXF_HOME} + instance_owner=${pxf_user} fi curl=`which curl` @@ -70,25 +70,25 @@ function fail() # function createInstance() { - mkdir -p $instance_root - mkdir -p $instance_root/$instance_name - cp -r $tomcat_root/* $instance_root/$instance_name/. + mkdir -p ${instance_root} + mkdir -p ${instance_root}/${instance_name} + cp -r ${tomcat_root}/* ${instance_root}/${instance_name}/. if [ $? -gt 0 ]; then echo ERROR: instance creation failed return 1 fi - if [ ! -z $pxf_user ]; then - chown -R $instance_owner $instance_root + if [ ! -z ${pxf_user} ]; then + chown -R ${instance_owner} ${instance_root} fi - chmod 700 $instance_root/$instance_name + chmod 700 ${instance_root}/${instance_name} # copy configuration files into instance - cp $tomcat_templates/bin/setenv.sh $instance_root/$instance_name/bin/setenv.sh - cp $tomcat_templates/conf/catalina.properties $instance_root/$instance_name/conf/. - cp $tomcat_templates/conf/server.xml $instance_root/$instance_name/conf/. - cp $tomcat_templates/conf/web.xml $instance_root/$instance_name/conf/. + cp ${tomcat_templates}/bin/setenv.sh ${instance_root}/${instance_name}/bin/setenv.sh + cp ${tomcat_templates}/conf/catalina.properties ${instance_root}/${instance_name}/conf/. + cp ${tomcat_templates}/conf/server.xml ${instance_root}/${instance_name}/conf/. + cp ${tomcat_templates}/conf/web.xml ${instance_root}/${instance_name}/conf/. return 0 } @@ -98,8 +98,8 @@ function createInstance() # function deployWebapp() { - cp $pxf_root/pxf.war $instance_root/$instance_name/webapps/ || return 1 - cp $pxf_root/pxf-service-*[0-9].jar $instance_root/$instance_name/lib/ || return 1 + cp ${pxf_root}/pxf.war ${instance_root}/${instance_name}/webapps/ || return 1 + cp ${pxf_root}/pxf-service-*[0-9].jar ${instance_root}/${instance_name}/lib/ || return 1 return 0 } @@ -124,7 +124,7 @@ function waitForTomcat() return 1 fi echo "tomcat not responding, re-trying after ${sleep_time} second (attempt number ${attempts})" - sleep $sleep_time + sleep ${sleep_time} done return 0 @@ -139,11 +139,11 @@ function checkWebapp() waitForTomcat $1 || return 1 echo Checking if PXF webapp is up and running... - curlResponse=$($curl -s "http://localhost:${instance_port}/pxf/v0") + curlResponse=$(${curl} -s "http://localhost:${instance_port}/pxf/v0") expectedResponse="Wrong version v0, supported version is v[0-9]+" - if [[ $curlResponse =~ $expectedResponse ]]; then - echo PXF webapp is up + if [[ ${curlResponse} =~ $expectedResponse ]]; then + echo PXF webapp is listening on port ${instance_port} return 0 fi @@ -159,7 +159,7 @@ function instanceExists() return 1 fi - $instance_root/$instance_name/bin/catalina.sh version > /dev/null 2>&1 + ${instance_root}/${instance_name}/bin/catalina.sh version > /dev/null 2>&1 return $? } @@ -168,7 +168,7 @@ function doInit() { instanceExists if [ $? -eq 0 ]; then - echo WARNING: instance already exists in $instance_root, nothing to do... + echo WARNING: instance already exists in ${instance_root}, nothing to do... return 0 fi determineHadoopDistro @@ -185,91 +185,74 @@ function doInit() # function configureWebapp() { - if [ -z $PXF_HOME ]; then + if [ -z ${PXF_HOME} ]; then # webapp doesn't require patch return 0 fi - pushd $instance_root/$instance_name/webapps || return 1 + pushd ${instance_root}/${instance_name}/webapps > /dev/null || return 1 rm -rf pxf mkdir pxf cd pxf unzip -q ../pxf.war - popd + popd > /dev/null - context_file=$instance_root/$instance_name/webapps/pxf/META-INF/context.xml - cat $context_file | \ - sed -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"$PXF_HOME\/conf\/pxf-private.classpath\":" \ - -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"$PXF_HOME\/conf\/pxf-public.classpath\":" > context.xml.tmp - mv context.xml.tmp $context_file + context_file=${instance_root}/${instance_name}/webapps/pxf/META-INF/context.xml + sed -i -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"${PXF_HOME}\/conf\/pxf-private.classpath\":" \ + -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"${PXF_HOME}\/conf\/pxf-public.classpath\":" ${context_file} - web_file=$instance_root/$instance_name/webapps/pxf/WEB-INF/web.xml - cat $web_file | \ - sed "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" > web.xml.tmp - mv web.xml.tmp $web_file + web_file=${instance_root}/${instance_name}/webapps/pxf/WEB-INF/web.xml + sed -i "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" ${web_file} # set port - catalinaProperties=$instance_root/$instance_name/conf/catalina.properties - cat $catalinaProperties | \ - sed "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" \ - > ${catalinaProperties}.tmp - - rm $catalinaProperties - mv ${catalinaProperties}.tmp $catalinaProperties + catalinaProperties=${instance_root}/${instance_name}/conf/catalina.properties + sed -i "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" ${catalinaProperties} # set container configs - catalinaEnv=$instance_root/$instance_name/bin/setenv.sh - cat $catalinaEnv | \ - sed -e "s|JVM_OPTS=.*$|JVM_OPTS=\"$PXF_JVM_OPTS\"|g" | \ - sed -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=$PXF_LOGDIR |g" | \ - sed -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=$PXF_RUNDIR/catalina.pid|g" | \ - sed -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=$PXF_LOGDIR/catalina.out|g" \ - > ${catalinaEnv}.tmp - rm $catalinaEnv - mv ${catalinaEnv}.tmp $catalinaEnv + catalinaEnv=${instance_root}/${instance_name}/bin/setenv.sh + sed -i -e "s|JVM_OPTS=.*$|JVM_OPTS=\"${PXF_JVM_OPTS}\"|g" \ + -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=${PXF_LOGDIR} |g" \ + -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=${PXF_RUNDIR}/catalina.pid|g" \ + -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=${PXF_LOGDIR}/catalina.out|g" ${catalinaEnv} # set log directories - catalinaLog=$instance_root/$instance_name/conf/logging.properties - cat $catalinaLog | \ - sed "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = $PXF_LOGDIR|g" \ - > ${catalinaLog}.tmp - rm $catalinaLog - mv ${catalinaLog}.tmp $catalinaLog + catalinaLog=${instance_root}/$instance_name/conf/logging.properties + sed -i "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = ${PXF_LOGDIR}|g" ${catalinaLog} } function commandWebapp() { command=$1 - pushd $instance_root - if [ ! -z $pxf_user ]; then + pushd ${instance_root} > /dev/null + if [ ! -z ${pxf_user} ]; then # Run command as a pxf_user - su $pxf_user -c "$instance_root/$instance_name/bin/catalina.sh $command" + su ${pxf_user} -c "$instance_root/$instance_name/bin/catalina.sh $command" else # Run command as a current user - $instance_root/$instance_name/bin/catalina.sh $command + ${instance_root}/${instance_name}/bin/catalina.sh ${command} fi if [ $? -ne 0 ]; then return 1 fi - popd + popd > /dev/null } function createLogsDir() { - mkdir -p $PXF_LOGDIR - if [ ! -z $pxf_user ]; then - chown -R $instance_owner $PXF_LOGDIR + mkdir -p ${PXF_LOGDIR} + if [ ! -z ${pxf_user} ]; then + chown -R ${instance_owner} ${PXF_LOGDIR} fi - chmod 700 $PXF_LOGDIR + chmod 700 ${PXF_LOGDIR} return 0 } function createRunDir() { - mkdir -p $PXF_RUNDIR - if [ ! -z $pxf_user ]; then - chown -R $instance_owner $PXF_RUNDIR + mkdir -p ${PXF_RUNDIR} + if [ ! -z ${pxf_user} ]; then + chown -R ${instance_owner} ${PXF_RUNDIR} fi - chmod 700 $PXF_RUNDIR + chmod 700 ${PXF_RUNDIR} return 0 } @@ -379,8 +362,8 @@ function validateParameters() fi # validate pxf user - if [ ! -z $pxf_user ]; then - id $pxf_user &> /dev/null; + if [ ! -z ${pxf_user} ]; then + id ${pxf_user} &> /dev/null; if [ "$?" -ne "0" ] then echo "ERROR: User $pxf_user doesn't exist. Please set valid user via \$PXF_USER variable" @@ -395,7 +378,7 @@ function validateParameters() fi # validate JAVA_HOME - if [ ! -x $JAVA_HOME/bin/java ]; then + if [ ! -x ${JAVA_HOME}/bin/java ]; then echo ERROR: \$JAVA_HOME is invalid exit 1 fi From 7f182be5e34abad4395c7ff093e45990154445b3 Mon Sep 17 00:00:00 2001 From: shivzone Date: Mon, 5 Feb 2018 15:54:20 -0800 Subject: [PATCH 3/6] HAWQ-1581. HAWQ pxf client fixes for parameter isolation --- src/backend/access/external/pxfuriparser.c | 6 +++--- src/backend/access/external/test/pxfuriparser_test.c | 4 ++-- src/bin/gpfusion/gpbridgeapi.c | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/access/external/pxfuriparser.c b/src/backend/access/external/pxfuriparser.c index 17d58036ce..595280cef6 100644 --- a/src/backend/access/external/pxfuriparser.c +++ b/src/backend/access/external/pxfuriparser.c @@ -530,7 +530,7 @@ GPHDUri_parse_option(char* pair, GPHDUri *uri) option_data->value = pnstrdup(sep + 1, value_len); char *x_gp_key = normalize_key_name(option_data->key); - if (strcmp(x_gp_key, "X-GP-PROFILE") == 0) + if (strcmp(x_gp_key, "X-GP-OPTIONS-PROFILE") == 0) { uri->profile = pstrdup(option_data->value); } @@ -899,7 +899,7 @@ bool RelationIsExternalPxfReadOnly(Relation rel, StringInfo location) /* * Full name of the HEADER KEY expected by the PXF service - * Converts input string to upper case and prepends "X-GP-" string + * Converts input string to upper case and prepends "X-GP-OPTIONS" string * */ char* normalize_key_name(const char* key) @@ -914,7 +914,7 @@ char* normalize_key_name(const char* key) StringInfoData formatter; initStringInfo(&formatter); char* upperCasedKey = str_toupper(pstrdup(key), strlen(key)); - appendStringInfo(&formatter, "X-GP-%s", upperCasedKey); + appendStringInfo(&formatter, "X-GP-OPTIONS-%s", upperCasedKey); pfree(upperCasedKey); return formatter.data; diff --git a/src/backend/access/external/test/pxfuriparser_test.c b/src/backend/access/external/test/pxfuriparser_test.c index 1912d2b54f..2efdf07c0f 100644 --- a/src/backend/access/external/test/pxfuriparser_test.c +++ b/src/backend/access/external/test/pxfuriparser_test.c @@ -883,7 +883,7 @@ test__normalize_key_name_Positive(void **state) { char *input_key = strdup("mIxEdCaSeVaLuE"); char *normalized_key = normalize_key_name(input_key); - assert_string_equal(normalized_key, "X-GP-MIXEDCASEVALUE"); + assert_string_equal(normalized_key, "X-GP-OPTIONS-MIXEDCASEVALUE"); pfree(input_key); pfree(normalized_key); @@ -894,7 +894,7 @@ test__normalize_key_name_PositiveUpperCase(void **state) { char *input_key = strdup("ALREADY_UPPER_CASE"); char *normalized_key = normalize_key_name(input_key); - assert_string_equal(normalized_key, "X-GP-ALREADY_UPPER_CASE"); + assert_string_equal(normalized_key, "X-GP-OPTIONS-ALREADY_UPPER_CASE"); pfree(input_key); pfree(normalized_key); diff --git a/src/bin/gpfusion/gpbridgeapi.c b/src/bin/gpfusion/gpbridgeapi.c index a853f069f3..2b5f221041 100644 --- a/src/bin/gpfusion/gpbridgeapi.c +++ b/src/bin/gpfusion/gpbridgeapi.c @@ -234,13 +234,13 @@ void set_current_fragment_headers(gphadoop_context* context) if (frag_data->profile) { /* if current fragment has optimal profile set it*/ - churl_headers_override(context->churl_headers, "X-GP-PROFILE", frag_data->profile); + churl_headers_override(context->churl_headers, "X-GP-OPTIONS-PROFILE", frag_data->profile); elog(DEBUG2, "pxf: set_current_fragment_headers: using profile: %s", frag_data->profile); } else if (context->gphd_uri->profile) { /* if current fragment doesn't have any optimal profile, set to use profile from url */ - churl_headers_override(context->churl_headers, "X-GP-PROFILE", context->gphd_uri->profile); + churl_headers_override(context->churl_headers, "X-GP-OPTIONS-PROFILE", context->gphd_uri->profile); elog(DEBUG2, "pxf: set_current_fragment_headers: using profile: %s", context->gphd_uri->profile); } /* if there is no profile passed in url, we expect to have accessor+fragmenter+resolver so no action needed by this point */ From c29c95796e7a16ad274b567dd95718c92c27be36 Mon Sep 17 00:00:00 2001 From: shivzone Date: Mon, 5 Feb 2018 15:55:04 -0800 Subject: [PATCH 4/6] HAWQ-1581. Fixed profile duplicate check --- .../org/apache/hawq/pxf/service/utilities/ProtocolData.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java index 0c9e531b05..ab085ca115 100644 --- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java +++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java @@ -215,7 +215,7 @@ private void checkForDuplicates(Map plugins, if (!duplicates.isEmpty()) { throw new IllegalArgumentException("Profile '" + profile + "' already defines: " - + String.valueOf(duplicates).replace("X-GP-", "")); + + String.valueOf(duplicates).replace("X-GP-OPTIONS-", "")); } } From bed06f9b73b2144cdc2df7c67f9cc82629d64216 Mon Sep 17 00:00:00 2001 From: shivzone Date: Tue, 6 Feb 2018 14:11:23 -0800 Subject: [PATCH 5/6] HAWQ-1581. ProtocolData Fix with protocolViolation check for profile --- .../pxf/service/utilities/ProtocolData.java | 4 ++-- pxf/pxf-service/src/scripts/pxf-service | 19 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java index ab085ca115..30e53ede1f 100644 --- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java +++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java @@ -94,11 +94,11 @@ public ProtocolData(Map paramsMap) { } accessor = getUserProperty("ACCESSOR"); if(accessor == null) { - protocolViolation(accessor); + protocolViolation("ACCESSOR"); } resolver = getUserProperty("RESOLVER"); if(resolver == null) { - protocolViolation(resolver); + protocolViolation("RESOLVER")); } fragmenter = getUserProperty("FRAGMENTER"); diff --git a/pxf/pxf-service/src/scripts/pxf-service b/pxf/pxf-service/src/scripts/pxf-service index d06127978e..13fcf44f69 100644 --- a/pxf/pxf-service/src/scripts/pxf-service +++ b/pxf/pxf-service/src/scripts/pxf-service @@ -20,7 +20,6 @@ # pxf-service start/stop/initialize/status the PXF instance # - if [ -z ${PXF_HOME} ]; then parent_script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )/.." && pwd )" env_script=${parent_script_dir}/conf/pxf-env.sh @@ -197,26 +196,26 @@ function configureWebapp() popd > /dev/null context_file=${instance_root}/${instance_name}/webapps/pxf/META-INF/context.xml - sed -i -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"${PXF_HOME}\/conf\/pxf-private.classpath\":" \ - -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"${PXF_HOME}\/conf\/pxf-public.classpath\":" ${context_file} + sed -i -e "s:classpathFiles=\"[a-zA-Z0-9\/\;.-]*\":classpathFiles=\"${PXF_HOME}\/conf\/pxf-private.classpath\":" ${context_file} + sed -i -e "s:secondaryClasspathFiles=\"[a-zA-Z0-9\/\;.-]*\":secondaryClasspathFiles=\"${PXF_HOME}\/conf\/pxf-public.classpath\":" ${context_file} web_file=${instance_root}/${instance_name}/webapps/pxf/WEB-INF/web.xml - sed -i "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" ${web_file} + sed -i -e "s:.*pxf-log4j.properties<\/param-value>:$PXF_HOME\/conf\/pxf-log4j.properties<\/param-value>:" ${web_file} # set port catalinaProperties=${instance_root}/${instance_name}/conf/catalina.properties - sed -i "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" ${catalinaProperties} + sed -i -e "s|^[[:blank:]]*connector.http.port=.*$|connector.http.port=$instance_port|g" ${catalinaProperties} # set container configs catalinaEnv=${instance_root}/${instance_name}/bin/setenv.sh - sed -i -e "s|JVM_OPTS=.*$|JVM_OPTS=\"${PXF_JVM_OPTS}\"|g" \ - -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=${PXF_LOGDIR} |g" \ - -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=${PXF_RUNDIR}/catalina.pid|g" \ - -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=${PXF_LOGDIR}/catalina.out|g" ${catalinaEnv} + sed -i -e "s|JVM_OPTS=.*$|JVM_OPTS=\"${PXF_JVM_OPTS}\"|g" ${catalinaEnv} + sed -i -e "s|-Dpxf.log.dir=[^[:space:]^\"]*|-Dpxf.log.dir=${PXF_LOGDIR} |g" ${catalinaEnv} + sed -i -e "s|^[[:blank:]]*CATALINA_PID=.*$|CATALINA_PID=${PXF_RUNDIR}/catalina.pid|g" ${catalinaEnv} + sed -i -e "s|^[[:blank:]]*CATALINA_OUT=.*$|CATALINA_OUT=${PXF_LOGDIR}/catalina.out|g" ${catalinaEnv} # set log directories catalinaLog=${instance_root}/$instance_name/conf/logging.properties - sed -i "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = ${PXF_LOGDIR}|g" ${catalinaLog} + sed -i -e "s|juli.FileHandler.directory\s*=.*$|juli.FileHandler.directory = ${PXF_LOGDIR}|g" ${catalinaLog} } function commandWebapp() From f9a8e6da84846800050f88e8a178052cc93687c2 Mon Sep 17 00:00:00 2001 From: shivzone Date: Tue, 6 Feb 2018 14:23:58 -0800 Subject: [PATCH 6/6] HAWQ-1581. ProtocolData Fix syntax error --- .../org/apache/hawq/pxf/service/utilities/ProtocolData.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java index 30e53ede1f..1688982116 100644 --- a/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java +++ b/pxf/pxf-service/src/main/java/org/apache/hawq/pxf/service/utilities/ProtocolData.java @@ -98,7 +98,7 @@ public ProtocolData(Map paramsMap) { } resolver = getUserProperty("RESOLVER"); if(resolver == null) { - protocolViolation("RESOLVER")); + protocolViolation("RESOLVER"); } fragmenter = getUserProperty("FRAGMENTER");