Skip to content

RDBC-1048 Fix silent data corruption bugs in Document Session#285

Open
redknightlois wants to merge 4 commits intoravendb:v7.2from
redknightlois:RDBC-1048
Open

RDBC-1048 Fix silent data corruption bugs in Document Session#285
redknightlois wants to merge 4 commits intoravendb:v7.2from
redknightlois:RDBC-1048

Conversation

@redknightlois
Copy link
Copy Markdown
Member

Issue link

https://issues.hibernatingrhinos.com/issue/RDBC-1048/Silent-data-corruption-bugs-in-Python-client-Document-Session

Additional description

Fix 9 bugs where the client produces wrong results without raising errors:

  • Cluster transaction validation accepts all command types due to Python or operator misuse (if x == A or B: is always True)
  • Compare exchange double-create in same session goes undetected (same or pattern)
  • Batch patch status always treated as CREATED/PATCHED (same or pattern)
  • WaitFor replication/index builder settings written to wrong fields (copy-paste from adjacent builders)
  • Batch results silently dropped after time series operations (break instead of continue)
  • update_session_after_save_changes was a no-op with a TODO comment (cluster TX index not propagated)

Type of change

  • Bug fix
  • Regression bug fix
  • Optimization
  • New feature

How risky is the change?

  • Low
  • Moderate
  • High
  • Not relevant

Backward compatibility

  • Non breaking change
  • Ensured. Please explain how has it been implemented?
  • Breaking change
  • Not relevant

Is it platform specific issue?

  • Yes. Please list the affected platforms.
  • No

Documentation update

  • This change requires a documentation update. Please mark the issue on YouTrack using Documentation Required tag.
  • No documentation update is needed

Testing by Contributor

  • Tests have been added that prove the fix is effective or that the feature works
  • Internal classes added to the test class (e.g. entity or index definition classes) have the lowest possible access modifier (preferable private)
  • It has been verified by manual testing
  • Existing tests verify the correct behavior

Testing by RavenDB QA team

  • This change requires a special QA testing due to possible performance or resources usage implications (CPU, memory, IO). Please mark the issue on YouTrack using QA Required
    tag.
  • No special testing by RavenDB QA team is needed

Is there any existing behavior change of other features due to this change?

  • Yes. Please list the affected features/subsystems and provide appropriate explanation
  • No

UI work

  • It requires further work in the Studio. Please mark the issue on YouTrack using Studio Required tag.
  • No UI work is needed

…d batch patch status handling

Cluster transaction validation accepted all command types silently, allowing unsupported operations (attachments, counters) through without error. Compare exchange double-create in the same session went undetected, sending conflicting commands to the server. Batch patch status handling treated every status as CREATED/PATCHED, causing unnecessary document reprocessing after SaveChanges.
…nges

Setting explicit replication timeouts, throw-on-timeout behavior, or specific index names through the WaitFor builders had no effect. The user's values were written to wrong fields (copy-paste from adjacent builders), while the actual fields stayed None and got filled with convention defaults. A 5-second explicit timeout became 15 seconds. Calling with no arguments crashed due to a stale None reference.
…agating cluster transaction index

When a SaveChanges batch contained a time series operation followed by other operations (PUT, PATCH), all results after the time series entry were silently skipped. Documents modified later in the same batch did not get their metadata updated, leaving change vectors and timestamps stale.

Additionally, the cluster transaction index was never propagated back to the document store after SaveChanges, breaking read-your-writes consistency across sessions in cluster deployments.
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.

1 participant