You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2018/02/16 16:02:04 UTC

[GitHub] trafodion pull request #1449: RMS infrastructure now reads the configured pi...

GitHub user selvaganesang opened a pull request:

    https://github.com/apache/trafodion/pull/1449

    RMS infrastructure now reads the configured pid Max from /proc/sys/ke…

    …rnel/pid_max
    
    but limits it to a maximum of 128K. It can be overridden by "PID_MAX" environment
    variable in ms.env.
    
    If the PID_MAX variable is set to 0, 64K is assumed to be configured pid max.
    If the PID_MAX variable is set to less than 32K, then 32K is assumed to be configured pid max.
    
    When a SQL process has a pin greater than the configured pid max, an error message
    "Pid , is higher than the configured pid max <configured_pid>" is written to
    the SQL log file and disables RMS for those processes. Node won't be brought down.
    
    "Get Statistics for rms " shows the the configured pid max.
    
    The environment variable to set the size of RMS shared segment size is now
    changed to RMS_SHARED_SEG_SIZE_MB.
    
    genms change for controlling pid_max and rms shared segment size

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/selvaganesang/trafodion trafodion-2963

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/trafodion/pull/1449.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1449
    
----
commit e7624aaaf8d1cad59186c07686404c652d0eb97b
Author: selvaganesang <se...@...>
Date:   2018-02-16T15:56:48Z

    RMS infrastructure now reads the configured pid Max from /proc/sys/kernel/pid_max
    but limits it to a maximum of 128K. It can be overridden by "PID_MAX" environment
    variable in ms.env.
    
    If the PID_MAX variable is set to 0, 64K is assumed to be configured pid max.
    If the PID_MAX variable is set to less than 32K, then 32K is assumed to be configured pid max.
    
    When a SQL process has a pin greater than the configured pid max, an error message
    "Pid , is higher than the configured pid max <configured_pid>" is written to
    the SQL log file and disables RMS for those processes. Node won't be brought down.
    
    "Get Statistics for rms " shows the the configured pid max.
    
    The environment variable to set the size of RMS shared segment size is now
    changed to RMS_SHARED_SEG_SIZE_MB.
    
    genms change for controlling pid_max and rms shared segment size

----


---

[GitHub] trafodion pull request #1449: RMS infrastructure now reads the configured pi...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1449#discussion_r168860580
  
    --- Diff: core/sql/cli/Globals.cpp ---
    @@ -176,10 +176,21 @@ void CliGlobals::init( NABoolean espProcess,
         cli_globals = this;
         int error;
         statsGlobals_ = (StatsGlobals *)shareStatsSegment(shmId_);
    -    if (statsGlobals_ == NULL
    -      || (statsGlobals_ != NULL && 
    -        statsGlobals_->getVersion() != StatsGlobals::CURRENT_SHARED_OBJECTS_VERSION_))
    +    NABoolean reportError = FALSE;
    +    char msg[256];;
    +    if (statsGlobals_ != NULL && myPin_ >= statsGlobals_->getConfiguredPidMax())
    +       reportError = TRUE;
    +    if ((statsGlobals_ == NULL)
    +      || ((statsGlobals_ != NULL) && 
    +        ((statsGlobals_->getVersion() != StatsGlobals::CURRENT_SHARED_OBJECTS_VERSION_) ||
    +        (myPin_ >= statsGlobals_->getConfiguredPidMax()))))
         {
    +      if (reportError) {
    --- End diff --
    
    Could this lead to a log storm? Imagine that thousands of SQL processes with Pids that are too high come and go. (If there is one, there is likely to be many.) Could we blow up our logs?


---

[GitHub] trafodion pull request #1449: RMS infrastructure now reads the configured pi...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/trafodion/pull/1449#discussion_r168861011
  
    --- Diff: core/sql/executor/ex_esp_frag_dir.cpp ---
    @@ -101,11 +101,23 @@ ExEspFragInstanceDir::ExEspFragInstanceDir(CliGlobals *cliGlobals,
       pid_ = phandle.getPin();
     
       tid_ = syscall(SYS_gettid);
    -  if (statsGlobals_ == NULL
    -    || (statsGlobals_ != NULL && 
    -      statsGlobals_->getVersion() != StatsGlobals::CURRENT_SHARED_OBJECTS_VERSION_))
    +  NABoolean reportError = FALSE;
    +  char msg[256];;
    +  if (statsGlobals_ != NULL && pid_ >= statsGlobals_->getConfiguredPidMax())
    +     reportError = TRUE;
    +  if ((statsGlobals_ == NULL)
    +     || ((statsGlobals_ != NULL) && 
    +        ((statsGlobals_->getVersion() != StatsGlobals::CURRENT_SHARED_OBJECTS_VERSION_) ||
    +        (pid_ >= statsGlobals_->getConfiguredPidMax()))))
       {
    +    if (reportError) {
    --- End diff --
    
    Same comment as earlier.


---

[GitHub] trafodion pull request #1449: [TRAFODION-2963] RMS infrastructure now reads ...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/trafodion/pull/1449


---