[GitHub] carbondata pull request #2239: [CARBONDATA-2408] Fix search mode master Sasl...

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185679917
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/store/SparkCarbonStore.scala ---
    @@ -110,8 +110,16 @@ class SparkCarbonStore extends MetaCachedCarbonStore {
       }
     
       def startSearchMode(): Unit = {
    +    LOG.info("Starting search mode master")
         master = new Master(session.sparkContext.getConf)
         master.startService()
    +    var count = 0
    +    while (master.isStarted().compareAndSet(false, false) && count < 5000) {
    +      LOG.info(s"Waiting search mode master to start, retrying $count times")
    +      Thread.sleep(10)
    +      count = count + 1;
    +    }
    +    LOG.info("Search mode master started")
    --- End diff --
   
    Need one more if check, if (count == 5000), should throw exception because it is failed to start master


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185680045
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/store/SparkCarbonStore.scala ---
    @@ -110,8 +110,16 @@ class SparkCarbonStore extends MetaCachedCarbonStore {
       }
     
       def startSearchMode(): Unit = {
    +    LOG.info("Starting search mode master")
         master = new Master(session.sparkContext.getConf)
         master.startService()
    +    var count = 0
    +    while (master.isStarted().compareAndSet(false, false) && count < 5000) {
    --- End diff --
   
    I think you can move this while loop into master.startService, so that master.startService is a blocking operation until it is success.  No need to expose master.isStarted method


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185680076
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -64,13 +65,29 @@ class Master(sparkConf: SparkConf, port: Int) {
     
       private val scheduler: Scheduler = new Scheduler
     
    +  /**
    +   * flag is the master status, default value is false, which means master not start now.
    +   * if it is true, then search mode master finished start.
    +   */
    +  private var flag: AtomicBoolean = new AtomicBoolean(false)
    --- End diff --
   
    move this variable into startService


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185683438
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/store/SparkCarbonStore.scala ---
    @@ -110,8 +110,16 @@ class SparkCarbonStore extends MetaCachedCarbonStore {
       }
     
       def startSearchMode(): Unit = {
    +    LOG.info("Starting search mode master")
         master = new Master(session.sparkContext.getConf)
         master.startService()
    +    var count = 0
    +    while (master.isStarted().compareAndSet(false, false) && count < 5000) {
    +      LOG.info(s"Waiting search mode master to start, retrying $count times")
    +      Thread.sleep(10)
    +      count = count + 1;
    +    }
    +    LOG.info("Search mode master started")
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185683794
 
    --- Diff: integration/spark2/src/main/scala/org/apache/carbondata/store/SparkCarbonStore.scala ---
    @@ -110,8 +110,16 @@ class SparkCarbonStore extends MetaCachedCarbonStore {
       }
     
       def startSearchMode(): Unit = {
    +    LOG.info("Starting search mode master")
         master = new Master(session.sparkContext.getConf)
         master.startService()
    +    var count = 0
    +    while (master.isStarted().compareAndSet(false, false) && count < 5000) {
    --- End diff --
   
    ok，I will try


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185684520
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -64,13 +65,29 @@ class Master(sparkConf: SparkConf, port: Int) {
     
       private val scheduler: Scheduler = new Scheduler
     
    +  /**
    +   * flag is the master status, default value is false, which means master not start now.
    +   * if it is true, then search mode master finished start.
    +   */
    +  private var flag: AtomicBoolean = new AtomicBoolean(false)
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4433/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5594/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4688/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5595/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4434/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4689/



---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185717770
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -80,9 +83,32 @@ class Master(sparkConf: SparkConf, port: Int) {
               rpcEnv = new NettyRpcEnvFactory().create(config)
               val registryEndpoint: RpcEndpoint = new Registry(rpcEnv, Master.this)
               rpcEnv.setupEndpoint("registry-service", registryEndpoint)
    +          if (isStarted.compareAndSet(false, false)) {
    --- End diff --
   
    I think you can set it directly


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185718131
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -80,9 +83,32 @@ class Master(sparkConf: SparkConf, port: Int) {
               rpcEnv = new NettyRpcEnvFactory().create(config)
               val registryEndpoint: RpcEndpoint = new Registry(rpcEnv, Master.this)
               rpcEnv.setupEndpoint("registry-service", registryEndpoint)
    +          if (isStarted.compareAndSet(false, false)) {
    +            synchronized {
    +              if (isStarted.compareAndSet(false, false)) {
    +                isStarted.set(true)
    +              }
    +            }
    +          }
               rpcEnv.awaitTermination()
             }
           }).start()
    +      var count = 0
    +      val countThreshold = 5000
    +      while (isStarted.compareAndSet(false, false) && count < countThreshold) {
    +        LOG.info(s"Waiting search mode master to start, retrying $count times")
    +        Thread.sleep(10)
    +        count = count + 1;
    +      }
    +      if (count >= countThreshold) {
    +        LOG.error(s"Search mode try $countThreshold times to start master but failed")
    +        throw new RuntimeException(
    +          s"Search mode try $countThreshold times to start master but failed")
    +      } else {
    +        LOG.info("Search mode master started")
    +      }
    +    } else {
    +      LOG.info("Search mode master has already existed before.")
    --- End diff --
   
    change to `Search mode master has already started`


---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185718247
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -70,7 +71,9 @@ class Master(sparkConf: SparkConf, port: Int) {
     
       /** start service and listen on port passed in constructor */
       def startService(): Unit = {
    +    val isStarted: AtomicBoolean = new AtomicBoolean(false)
    --- End diff --
   
    move it into next line


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185725067
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -70,7 +71,9 @@ class Master(sparkConf: SparkConf, port: Int) {
     
       /** start service and listen on port passed in constructor */
       def startService(): Unit = {
    +    val isStarted: AtomicBoolean = new AtomicBoolean(false)
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185725112
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -80,9 +83,32 @@ class Master(sparkConf: SparkConf, port: Int) {
               rpcEnv = new NettyRpcEnvFactory().create(config)
               val registryEndpoint: RpcEndpoint = new Registry(rpcEnv, Master.this)
               rpcEnv.setupEndpoint("registry-service", registryEndpoint)
    +          if (isStarted.compareAndSet(false, false)) {
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user xubo245 commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2239#discussion_r185725441
 
    --- Diff: store/search/src/main/scala/org/apache/spark/rpc/Master.scala ---
    @@ -80,9 +83,32 @@ class Master(sparkConf: SparkConf, port: Int) {
               rpcEnv = new NettyRpcEnvFactory().create(config)
               val registryEndpoint: RpcEndpoint = new Registry(rpcEnv, Master.this)
               rpcEnv.setupEndpoint("registry-service", registryEndpoint)
    +          if (isStarted.compareAndSet(false, false)) {
    +            synchronized {
    +              if (isStarted.compareAndSet(false, false)) {
    +                isStarted.set(true)
    +              }
    +            }
    +          }
               rpcEnv.awaitTermination()
             }
           }).start()
    +      var count = 0
    +      val countThreshold = 5000
    +      while (isStarted.compareAndSet(false, false) && count < countThreshold) {
    +        LOG.info(s"Waiting search mode master to start, retrying $count times")
    +        Thread.sleep(10)
    +        count = count + 1;
    +      }
    +      if (count >= countThreshold) {
    +        LOG.error(s"Search mode try $countThreshold times to start master but failed")
    +        throw new RuntimeException(
    +          s"Search mode try $countThreshold times to start master but failed")
    +      } else {
    +        LOG.info("Search mode master started")
    +      }
    +    } else {
    +      LOG.info("Search mode master has already existed before.")
    --- End diff --
   
    ok


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4445/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2239
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5606/



---