New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xSQLServerMemory: Should Be Cluster Aware #867

Closed
randomnote1 opened this Issue Oct 13, 2017 · 7 comments

Comments

2 participants
@randomnote1
Contributor

randomnote1 commented Oct 13, 2017

Details of the scenario you tried and the problem that is occurring:
When the SQL Instance is an FCI and the configuration is pushed to every node of the FCI, each of the resources that reach into the Instance execute multiple times per "cycle".

For example, if I have a 2-node FCI and have xSQLServerMemory defined, both nodes will reach into the instance and set the memory regardless if it is the owner node. This could result in the server memory being configured twice every 15 minutes.

In the case where new servers are being added to the FCI that have a different memory configuration, this will result in the new node's configuration fighting with the old nodes causing the memory configuration to flip-flop between the correct memory allocations for each server. This scenario has been observed in a real-life environment when using dynamic memory allocation.

The DSC configuration that is using the resource (as detailed as possible):
The following configuration is applied on two or more nodes of a Failover Cluster Instance (FCI):

node localhost
{
    xSQLServerMemory Set_SQLServerMaxMemory_ToAuto
    {
        Ensure               = 'Present'
        DynamicAlloc         = $true
        SQLServer            = 'SQLServer'
        SQLInstanceName      = 'DSC'
        PsDscRunAsCredential = $SysAdminAccount
    }
}

Version of the Operating System, SQL Server and PowerShell the DSC Target Node is running:
Any

What module (SqlServer or SQLPS) and which version of the module the DSC Target Node is running:
Any

Version of the DSC module you're using, or 'dev' if you're using current dev branch:
Any

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Oct 13, 2017

Contributor

I propose that if the node that is executing the configuration is not the active owner of the FCI, it should write a verbose message stating that, and then silently exit.

Contributor

randomnote1 commented Oct 13, 2017

I propose that if the node that is executing the configuration is not the active owner of the FCI, it should write a verbose message stating that, and then silently exit.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Oct 13, 2017

Contributor

I can see this being a problem, where you have two different memory configurations in 2-or-more-node cluster. I agree that in those cases the user need to specify the same configuration in all the nodes. and that the Test-TargetResource should silently exit after writing a verbose message.

Test-DscConfiguration will always return $true since it silently exists. It will expect that the configuration is in desired state when the node does not own the instance.

But...

  • Should this behavior only apply when configuration is dynamic allocation?
  • How should the Get-TargetResource do when Get-DscConfiguration is called? It should go out an fetch the actual memory configuration?
Contributor

johlju commented Oct 13, 2017

I can see this being a problem, where you have two different memory configurations in 2-or-more-node cluster. I agree that in those cases the user need to specify the same configuration in all the nodes. and that the Test-TargetResource should silently exit after writing a verbose message.

Test-DscConfiguration will always return $true since it silently exists. It will expect that the configuration is in desired state when the node does not own the instance.

But...

  • Should this behavior only apply when configuration is dynamic allocation?
  • How should the Get-TargetResource do when Get-DscConfiguration is called? It should go out an fetch the actual memory configuration?
@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Oct 13, 2017

Contributor

Those are excellent questions.

I would hope that if static memory allocations are defined adequate memory would be available on all nodes. However, in reality I can see scenarios where the secondary node may be shorted resources due to budgetary constraints (purely non-technical reasons). I think that memory is node specific, so we should allow for different configurations per node.

For the get, is it important to evaluate the validity of a component that is not currently present on the node? I would argue no. Therefore, I think get should not bother gathering the memory configuration if the current node is not hosting the instance.

Contributor

randomnote1 commented Oct 13, 2017

Those are excellent questions.

I would hope that if static memory allocations are defined adequate memory would be available on all nodes. However, in reality I can see scenarios where the secondary node may be shorted resources due to budgetary constraints (purely non-technical reasons). I think that memory is node specific, so we should allow for different configurations per node.

For the get, is it important to evaluate the validity of a component that is not currently present on the node? I would argue no. Therefore, I think get should not bother gathering the memory configuration if the current node is not hosting the instance.

@johlju

This comment has been minimized.

Show comment
Hide comment
@johlju

johlju Oct 14, 2017

Contributor

I could agree on supporting different configurations for both static and dynamic memory allocation. For a static memory allocation, then the state would practically never be in desired state for the node not owning the instance.

Regarding Get. I understand your point, and can agree on it, but that would make it stray from normal behavior for a resource. It there were something dependent on the configuration from the get (like an automation) and all of the sudden the resource return null values it would be very strange, and that node would not feel like it would be in desired state. I rather see Get always getting the current values from the instance regardless of ownership.

So, to conclude. I agree this need to be done for both dynamic and static memory allocation. But in my opinion we should make Get-TargetResource always returning current state.

Contributor

johlju commented Oct 14, 2017

I could agree on supporting different configurations for both static and dynamic memory allocation. For a static memory allocation, then the state would practically never be in desired state for the node not owning the instance.

Regarding Get. I understand your point, and can agree on it, but that would make it stray from normal behavior for a resource. It there were something dependent on the configuration from the get (like an automation) and all of the sudden the resource return null values it would be very strange, and that node would not feel like it would be in desired state. I rather see Get always getting the current values from the instance regardless of ownership.

So, to conclude. I agree this need to be done for both dynamic and static memory allocation. But in my opinion we should make Get-TargetResource always returning current state.

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Oct 14, 2017

Contributor

Makes sense to me!

Contributor

randomnote1 commented Oct 14, 2017

Makes sense to me!

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Oct 20, 2017

Contributor

To resolve this issue, a parameter called ProcessOnlyOnActiveNode should be created on the Set-TargetResource and Test-TargetResource functions.

The Get-TargetResource function will execute the helper function Test-ActiveNode to determine if the current node is actively hosting the SQL instance and return the result via the read-only property IsActiveNode.

The Test-TargetResource function will use the value of ProcessOnlyOnActiveNode and IsActiveNode to determine if it should continue. If ProcessOnlyOnActiveNode is $true and IsActiveNode is $false, Test-TargetResource will return $true.

I put together this gist as a guide to updating the resource.

Also, see the xSQLServerAlwaysOnAvailabilityGroup resource for a completed example.

Contributor

randomnote1 commented Oct 20, 2017

To resolve this issue, a parameter called ProcessOnlyOnActiveNode should be created on the Set-TargetResource and Test-TargetResource functions.

The Get-TargetResource function will execute the helper function Test-ActiveNode to determine if the current node is actively hosting the SQL instance and return the result via the read-only property IsActiveNode.

The Test-TargetResource function will use the value of ProcessOnlyOnActiveNode and IsActiveNode to determine if it should continue. If ProcessOnlyOnActiveNode is $true and IsActiveNode is $false, Test-TargetResource will return $true.

I put together this gist as a guide to updating the resource.

Also, see the xSQLServerAlwaysOnAvailabilityGroup resource for a completed example.

@randomnote1

This comment has been minimized.

Show comment
Hide comment
@randomnote1

randomnote1 Oct 27, 2017

Contributor

Starting this next

Contributor

randomnote1 commented Oct 27, 2017

Starting this next

@johlju johlju added in progress and removed help wanted labels Oct 27, 2017

@johlju johlju moved this from Backlog to In progress in Resources should be cluster aware Oct 27, 2017

@randomnote1 randomnote1 referenced this issue Oct 30, 2017

Merged

xSQLServerMemory: Make Cluster Aware #896

5 of 5 tasks complete

@johlju johlju closed this in #896 Oct 31, 2017

johlju added a commit that referenced this issue Oct 31, 2017

xSQLServerMemory: Make Cluster Aware (#896)
- Changes to xSQLServerMemory
  - Made the resource cluster aware. When ProcessOnlyOnActiveNode is specified,
    the resource will only determine if a change is needed if the target node
    is the active host of the SQL Server instance (issue #867).

@johlju johlju removed the in progress label Oct 31, 2017

@johlju johlju moved this from In progress to Done in Resources should be cluster aware Oct 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment