Skip to content

Conversation

@lukasz-antoniak
Copy link
Member

No description provided.

@absurdfarce
Copy link
Contributor

Kicked off a Jenkins build of this to confirm we're okay, will post updates here when that completes.

@absurdfarce
Copy link
Contributor

Bah, that doesn't make any sense... we won't use this Jenkinsfile, and without that we don't get testing on the new platforms.

Might wind up just merging these fixes and cleaning up anything which still looks out of whack.

Copy link
Contributor

@absurdfarce absurdfarce left a comment

Choose a reason for hiding this comment

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

Looks quite good! Think we still need a bit more work re: how we include HCD versions into the existing set of matrices but that's the only thing we need to do at this point.

Jenkinsfile Outdated
"SERVER": DEFAULT_HCD,
"RUNTIME": DEFAULT_RUNTIME,
"CYTHON": DEFAULT_CYTHON
],
Copy link
Contributor

@absurdfarce absurdfarce Oct 14, 2024

Choose a reason for hiding this comment

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

Discussed last week but the default matrix used for most test builds is the "SMOKE" matrix... and it seems to me we'd prefer to bring HCD into that matrix rather than introducing a new matrix for HCD (which would have to be explicitly triggered). To that end this PR should:

  • Remove the "HCD" matrix above
  • Either add HCD versions to the DEFAULT_DSE array or keep them in the DEFAULT_HCD array
  • Update the server selection logic below (or here) to include HCD versions in some way.

There is some subtlety here. We're already pulling the most recent DSE version for smoke tests via the DEFAULT_DSE.takeRight(1) call. What we probably want to wind up with is smoke tests running against the current DSE minor (6.9.x), the previous DSE minor (6.8.x) and HCD. You can make this happen whether you keep a separate array for HCD or fold it into the DEFAULT_DSE array.

Keep in mind that every new server backend adds 2 x (number of Python versions) builds to the Jenkins runs... so we want to be judicious about what we add here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good catch. Fixed.

if exception_type == "NoHostAvailable":
self.fail("PYTHON-91: Disconnected from Cassandra: %s" % result.message)
if exception_type in ["WriteTimeout", "WriteFailure", "ReadTimeout", "ReadFailure", "ErrorMessageSub"]:
if exception_type in ["WriteTimeout", "WriteFailure", "ReadTimeout", "ReadFailure", "ErrorMessageSub", "ErrorMessage"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know why we're suddenly seeing the parent class ErrorMessage here when we weren't before?

Copy link
Member Author

Choose a reason for hiding this comment

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

Driver receives exceptions:

WriteTimeout('Error from server: code=1100 [Coordinator node timed out waiting for replica nodes\' responses] message="CAS operation timed out - encountered contentions: 18" info={\'consistency\': \'SERIAL\', \'required_responses\': 2, \'received_responses\': 0, \'write_type\': \'CAS\'}')

ErrorMessage: <Error from server: code=1700 [Unknown] message="CAS operation result is unknown - proposal accepted by 1 but not a quorum.">

It looks clear here why CAS_WRITE_UNKNOWN is wrapped in ErrorMessage . Cannot answer why this was not the case before.

I have asked HCD engineers around, but they are not familiar with LWT changes introduced in C* 5.0.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for checking into it. For the record I don't think this difference should hold up getting this PR in. It seemed like an odd change when looking this over but I can envision perfectly good reasons why it might be the case.

@absurdfarce absurdfarce changed the title Run integration tests with HCD 1.0.0 PYTHON-1402 Support running test suite with HCD 1.0.0 Oct 15, 2024
],
"SMOKE": [
"SERVER": DEFAULT_CASSANDRA.takeRight(2) + DEFAULT_DSE.takeRight(1),
"SERVER": DEFAULT_CASSANDRA.takeRight(2) + DEFAULT_DSE.takeRight(2) + DEFAULT_HCD.takeRight(1),
Copy link
Contributor

Choose a reason for hiding this comment

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

For the record: I had this wrong in my earlier comment. We should actually be adding only four new builds to our smoke test build matrix with this change: 2 new backends * 2 Python versions (oldest and newest). That should be reasonably manageable. More to the point it's hard to imagine how we'd remove either 6.8.x, 6.9.x or HCD from our test matrix.

@absurdfarce absurdfarce merged commit 1b335d4 into apache:master Oct 16, 2024
2 checks passed
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.

2 participants