Skip to content

Conversation

Scooletz
Copy link

@Scooletz Scooletz commented Sep 24, 2025

https://issues.hibernatingrhinos.com/issue/RDoc-3516

This PR adjusts the information about SessionContext. It aims in providing more real-life scenarios and amend When To Use sections

@Scooletz Scooletz marked this pull request as draft September 24, 2025 08:33
// Assign a method that sets the default context string
// This string will be used for sessions that do Not provide a context string
// This method will be used for sessions that do Not have their context string set explicitly
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. This method => This default string
    The important point is that it’s not the method itself that’s used, but its return value - the default context string.

  2. The alternative phrasing (have their context string set explicitly) is 'passive voice' and adds unnecessary complexity. It’s less clear than the original
    Better to stick with:
    // This default string will be used for sessions that do Not provide a context string

  3. Another alternative (if you wanna retain 'method'):
    // This method will be called to provide a default context string for sessions that do not set one explicitly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot add a comment above, so dding here
Need to add a line break between lines #63 & #64
(The current rendering does not place line 64 on a new line)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mention of the default string is based on the fact that the method currently returns just a string and is not context aware. I do agree with the passive voice being too complex. I think though, that if you set the load balance behavior, you should provide something else here (the other discussion) than just a default string.

Copy link
Member

@Danielle9897 Danielle9897 Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So use 3'rd suggestion for the comment:
// This method will be called to provide a default context string for sessions that do not set one explicitly

}
// Return the context set elsewhere or default to the dbName
return (ctx.Items[ContextKey] as string) ?? dbName;
Copy link
Member

@Danielle9897 Danielle9897 Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I see the intention behind this version,
    but I think we should keep the original example as the default/basic one,
    and move this more advanced HttpContext-based logic to a separate usage example.
    This is a specific use case tied to running in a web application.
    The default example should be generic so it applies to all environments (e.g. console apps, services, etc.).

  2. Using "RavenDB.Context" as the key implies that it's something officially defined or required by RavenDB, which is Not the case. Better use something like:
    const string DefaultContextStringKey = "DefaultContextString";
    This makes it clear that it’s just a key of your choice, not something provided by RavenDB.
    Also, the name more accurately reflects what it stores - the context string value, not the entire context.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I don't agree with the reasoning. The previous version was returning a constant value that, if you use context is never the case. The example was simple but was not correct. In this version, yes, I agree that it's environment specific, but it shows the intent in a right way: you have a context, and you want it to be used for the balancing.
  2. Sure, this can be amended.

when a set of sessions handle a specific set of documents or similar data.
Load balancing can be achieved by routing requests from the sessions that handle similar topics to the same node, while routing other sessions to other nodes.
* When a session handles a specific set of documents or data, that can be scoped under a shared context.
Setting the context can greatly reduce chances for conflicts. It can be useful, when used with the [optimistic concurrency](../../../client-api/session/configuration/how-to-enable-optimistic-concurrency/),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicts happen only on writes, while the load-balancing behavior applies to both reads and writes.
The main benefit is consistent routing of related sessions (by team, user, or tenant) to the same node,
which spreads the overall load more effectively.
Conflict reduction when using optimistic concurrency can be mentioned as a secondary benefit.

========= Use this text: =========

When to use

* Distributing _Read and Write_ requests among cluster nodes can be beneficial
  when a set of sessions consistently handles a specific group of documents or related data.
  By setting a context string per session, you can ensure that similar sessions 
  (e.g., sessions for a specific user, tenant, or team) are routed to the same node, 
  while other sessions are routed to different nodes - achieving effective load distribution.
 
 * In scenarios where [optimistic concurrency](...) is enabled,  
  routing sessions for the same entity to the same node can help reduce _Write_ conflicts,  
  since concurrent updates tend to be coordinated on the same node.
  The node handling the write is also more likely to have the latest version of the document,  
  which helps avoid cross-node conflicts.

* Once load balancing is configured to use the session context,
  if many sessions still end up being routed to the same node,
  you can introduce additional distribution by adjusting the context seed on the store.

@Scooletz Scooletz self-assigned this Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants