[ https://issues.apache.org/jira/browse/YARN-11296?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17602483#comment-17602483 ]
ASF GitHub Bot commented on YARN-11296: --------------------------------------- goiri commented on code in PR #4858: URL: https://github.com/apache/hadoop/pull/4858#discussion_r967322160 ########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/SQLServer/FederationStateStoreTables.sql: ########## @@ -140,7 +140,7 @@ IF NOT EXISTS ( SELECT * FROM [FederationStateStore].sys.tables CREATE TABLE [dbo].[reservationsHomeSubCluster]( reservationId VARCHAR(128) COLLATE Latin1_General_100_BIN2 NOT NULL, homeSubCluster VARCHAR(256) NOT NULL, - createTime DATETIME2 NOT NULL CONSTRAINT ts_createAppTime DEFAULT GETUTCDATE(), + createTime DATETIME2 NOT NULL CONSTRAINT ts_createResTime DEFAULT GETUTCDATE(), Review Comment: This is also used in line 37. Where does this field come from? ########## hadoop-yarn-project/hadoop-yarn/bin/FederationStateStore/MySQL/FederationStateStoreTables.sql: ########## @@ -36,7 +36,7 @@ CREATE TABLE membership( state varchar(32) NOT NULL, lastStartTime bigint NULL, capability varchar(6000), - CONSTRAINT pk_subClusterId PRIMARY KEY (subClusterId) + CONSTRAINT pk_subClusterId PRIMARY KEY (subClusterId), Review Comment: Is this validated by the unit test? ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/sql/TestFDMySQLAccuracy.java: ########## @@ -0,0 +1,73 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.hadoop.yarn.server.federation.store.sql; + +import org.apache.hadoop.yarn.conf.YarnConfiguration; +import org.apache.hadoop.yarn.server.federation.store.FederationStateStore; +import org.apache.hadoop.yarn.server.federation.store.impl.MySQLFederationStateStore; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.sql.SQLException; +import java.util.List; + +public class TestFDMySQLAccuracy extends FederationSQLAccuracyTest { Review Comment: What is FD? ########## hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-common/src/test/java/org/apache/hadoop/yarn/server/federation/store/impl/SQLServerFederationStateStore.java: ########## @@ -0,0 +1,114 @@ +/** + * 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.hadoop.yarn.server.federation.store.impl; + +import org.apache.commons.io.FileUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.yarn.exceptions.YarnException; +import org.apache.hadoop.yarn.server.federation.store.FederationStateStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.sql.Connection; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * SQLServerFederationStateStore implementation of {@link FederationStateStore}. + */ +public class SQLServerFederationStateStore extends SQLFederationStateStore { + + private static final Logger LOG = + LoggerFactory.getLogger(SQLServerFederationStateStore.class); + + private Connection conn; + + private String createTableScriptPath; + + private static String createProcedureScriptPath; + + private List<String> tables = new ArrayList<>(); + + private List<String> procedures = new ArrayList<>(); + + @Override + public void init(Configuration conf) { + try { + super.init(conf); + + // get the sql that creates the table + createTableScriptPath = "." + File.separator + "target" + File.separator + + "test-classes" + File.separator + "SQLServer/FederationStateStoreTables.sql"; + LOG.info("createTableScriptPath >> {}", createTableScriptPath); + String createTableSQL = FileUtils.readFileToString(new File(createTableScriptPath), + StandardCharsets.UTF_8); + Pattern p = Pattern.compile("IF NOT EXISTS.*\\n(.*\\n){0,50}.*GO"); + Matcher m = p.matcher(createTableSQL); + while(m!=null && m.find()) { + tables.add(m.group()); + } + + // get the sql that creates the stored procedure + createProcedureScriptPath = "." + File.separator + "target" + File.separator + + "test-classes" + File.separator + "SQLServer/FederationStateStoreStoreProcs.sql"; + String createProcedureSQL = FileUtils.readFileToString(new File(createProcedureScriptPath), + StandardCharsets.UTF_8); + String[] results = createProcedureSQL.split("GO"); + for (String result : results) { + if (StringUtils.contains(result, "CREATE PROCEDURE")) { + procedures.add(result); + } + } + + LOG.info("SqlServer - tables = {}, procedures = {}", tables.size(), procedures.size()); + + conn = super.conn; + } catch (YarnException | IOException e1) { + LOG.error("ERROR: failed to init HSQLDB " + e1.getMessage()); + } + } + + public void closeConnection() { Review Comment: Many things here are repeated for SQLServer and MySQL. Can we move to the parent? > Fix SQLFederationStateStore#Sql script bug > ------------------------------------------ > > Key: YARN-11296 > URL: https://issues.apache.org/jira/browse/YARN-11296 > Project: Hadoop YARN > Issue Type: Bug > Components: federation > Affects Versions: 3.4.0 > Reporter: fanshilun > Assignee: fanshilun > Priority: Major > Labels: pull-request-available > > *MySQL:* > FederationStateStoreTables.sql cannot create *membership* table, both in > MySQL 5.7 and MySQL 8.0. > > {code:java} > CREATE TABLE membership( > subClusterId varchar(256) NOT NULL, > amRMServiceAddress varchar(256) NOT NULL, > clientRMServiceAddress varchar(256) NOT NULL, > rmAdminServiceAddress varchar(256) NOT NULL, > rmWebServiceAddress varchar(256) NOT NULL, > lastHeartBeat datetime NOT NULL, > state varchar(32) NOT NULL, > lastStartTime bigint NULL, > capability varchar(6000), > CONSTRAINT pk_subClusterId PRIMARY KEY (subClusterId), -- missing comma > UNIQUE(lastStartTime) > ); {code} > > *SQLServer:* > FederationStateStoreTables.sql > > {code:java} > IF NOT EXISTS ( SELECT * FROM [FederationStateStore].sys.tables > WHERE name = 'membership' > AND schema_id = SCHEMA_ID('dbo')) > BEGIN > PRINT 'Table membership does not exist, create it...' > SET ANSI_NULLS ON > SET QUOTED_IDENTIFIER ON > SET ANSI_PADDING ON > CREATE TABLE [dbo].[membership]( > [subClusterId] VARCHAR(256) COLLATE > Latin1_General_100_BIN2 NOT NULL, > [amRMServiceAddress] VARCHAR(256) NOT NULL, > [clientRMServiceAddress] VARCHAR(256) NOT NULL, > [rmAdminServiceAddress] VARCHAR(256) NOT NULL, > [rmWebServiceAddress] VARCHAR(256) NOT NULL, > [lastHeartBeat] DATETIME2 NOT NULL, > [state] VARCHAR(32) NOT NULL, > [lastStartTime] BIGINT NOT NULL, > [capability] VARCHAR(6000) NOT NULL, > CONSTRAINT [pk_subClusterId] PRIMARY KEY > ( > [subClusterId] > ), -- missing comma > CONSTRAINT [uc_lastStartTime] UNIQUE > ( > [lastStartTime] > ) > ) > SET ANSI_PADDING OFF > PRINT 'Table membership created.' > END > ELSE > PRINT 'Table membership exists, no operation required...' > GO > GO {code} > > -- This message was sent by Atlassian Jira (v8.20.10#820010) --------------------------------------------------------------------- To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org