[JAVA-2704] Remove protocol v5 beta status, add v6-beta #1529
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adutra
reviewed
Feb 8, 2021
driver-core/src/main/java/com/datastax/driver/core/ProtocolVersion.java
Outdated
Show resolved
Hide resolved
|
|
||
| private static final Message.ProtocolEncoder REQUEST_ENCODER = | ||
| new Message.ProtocolEncoder(ProtocolVersion.V5); | ||
| new Message.ProtocolEncoder(ProtocolVersion.V6); |
Contributor
There was a problem hiding this comment.
We could have left V5 here imo as it is more important to test the encoding of segments in V5 than in V6. Same in SegmentToFrameDecoderTest.
Contributor
There was a problem hiding this comment.
Thanks! Would you mind reverting SegmentToFrameDecoderTest as well?
Contributor
|
Also, I would require an entry to the changelog. Basically this: |
Author
adutra
approved these changes
Feb 16, 2021
Contributor
|
Thanks again @beobal. Indeed the right ticket number is JAVA-2705. This looks good to go, however our integration tests are failing with: This is with Cassandra 4.0-beta4. However my understanding of the situation is that 4.0-beta4 should include both CASSANDRA-15299 and CASSANDRA-16319 (the latter having promoted v5 out of beta). But the server logs show: So I'm puzzled... Do you know by chance what we are doing wrong in our tests? Is there a more recent version we should be using instead? |
Author
|
Hey @adutra. Yep, the change to promote v5 out of beta is not in 4.0-beta4. The ticket for that is CASSANDRA-14973, and there's a branch you can use for testing here |
Contributor
|
Thanks @beobal for the explanations. I confirmed the tests pass against your branch. |
dgarcia360
added a commit
to dgarcia360/java-driver
that referenced
this pull request
Nov 29, 2022
* Fix ProtocolBetaVersionTest * Make client encryption non optional when enabling SSL in CCMBridge * JAVA-2860: Avoid NPE if channel initialization crashes * [maven-release-plugin] prepare release 3.10.2 * [maven-release-plugin] prepare for next development iteration * Rebrand Apollo to Astra (apache#1507) Co-authored-by: Madhavan Sridharan <madhavan.sridharan@datastax.com> * JAVA-2705: Remove protocol v5 beta status, add v6-beta (apache#1529) * Remove incorrect assertion It's legal for a client to not supply a specific version, e.g. not call Cluster.Builder::withProtocolVersion, in which case factory.protocolVersion is null. While v5 was marked beta this wasn't a problem as Cluster.Builder::allowProtocolVersion was required, but now this causes an NPE in the driver. * Add support for Cassandra 4.0 to CCMBridge * JAVA-2923: Detect and use Guava's new HostAndPort.getHost method (apache#1533) * Fix failing test against C* 4.0 * JAVA-2922: Switch to modern framing format inside a channel handler (apache#1532) * JAVA-2924: Consider protocol version unsupported when server requires USE_BETA flag for it (apache#1534) * Minor improvements to CCM tests * Update version in docs * [maven-release-plugin] prepare release 3.11.0 * [maven-release-plugin] prepare for next development iteration * Fix wrong version in POM snippet * Update protocol versions compatibility matrix * Fix broken link to driver 3.x manual page * Minor change to HTML table layout * Remove broken link to binary tarball * Update log message to inform about incorrect CL for read repair (apache#1568) * JAVA-2967: Support native_transport_(address|port) + native_transport_port_ssl for DSE 6.8 (apache#1573) * Upgrades Sphinx Theme to version 1.x * Update .github/workflows/docs-pages@v2.yaml Co-authored-by: David Garcia <hi@davidgarcia.dev> * Update .github/workflows/docs-pr@v1.yaml Co-authored-by: David Garcia <hi@davidgarcia.dev> * Update .github/workflows/docs-pages@v2.yaml Co-authored-by: David Garcia <hi@davidgarcia.dev> * Update docs/source/conf.py Co-authored-by: David Garcia <hi@davidgarcia.dev> * Lint docs workflows * Add contribute button * Fix borrowConnection handling of missing connection to a shard Previously, if there was no connection to a shard and host.convictionPolicy.canReconnectNow() was true, method would wait for the new connection to be established. This is not correct because we would like to use connection to a different shard if available. This change fixes the problem. * Limit the rate at which HostConnectionPool opens connections Changed test: New reconnection mechanism caused one test to fail. Test needed to be changed, because missing connections will now be spawned by the reconnection mechanism - no need to send requests to schedule connection creation. * Make it possible to create connection using different server port This will be used to establish connection to the shard-aware port. * Fetch server ports with advanced shard awareness support Driver needs to fetch shard-aware port on first connection to node, so that subsequent connection can be shard aware. There is a separate shard-aware port for SSL connections, just as there is one for non shard-aware connections. ShardingInfo needs to expose the right port, depending on whether SSL is enabled or not. * Make advanced shard awareness configurable Create public API to configure port-based shard awareness. User can disable it with Cluster.Builder.withoutAdvancedShardAwareness() and set range of local ports used with Cluster.Builder.withLocalPortRange(int low, int high). * Implement PortAllocator PortAllocator finds a free port for a given shard in a given range. It will be used by port-based shard awareness. Important think to remember is that returned port may be stolen before we can use it, so to correctly bind to a local port we need to, in a loop, get a free port and try to bind it. * Make it possible to connect to given shard in Connection * Retry connection when local port is busy Local port may be stolen before we open channel, in that case we need to try again. * Use port-based shard awareness Use advanced (port-based) shard awareness both when creating initial connection pool, and later when creating missing connections. * Implement fallback to non-advanced shard awareness When we connect to different shard than requested, then probably NAT is present - we should disable advanced shard awareness and use old strategy. When we failed to connect to node using shard-aware port, then we disable advanced shard awareness for 5 minutes - hopefully connection problems will be fixed by then. * Pick unique ports during single build() call CCMBridge Builder will now attempt to re-randomize if current free port has already been returned for different service. This works under assumption that system will pick random ephemeral port upon calling new ServerSocket(0);. Fixes scylladb#99. * Run integration tests on all scylla-3.*x branches Change the GitHub Actions workflow to run on all branches that match scylla-3.*x glob pattern, not only scylla-3.x. This makes it easier to test the changes on specific version branches (such as scylla-3.10.2.x) before merging them into (main) scylla-3.x. * Fossa scanning (apache#1579) * Throw UnsupportedProtocolVersion handling OPTIONS Previously, if an OPTIONS packet was sent with a wrong protocol version, onOptionsResponse handler method would return a generic TransportException exception. This patch implements a similar approach to onStartupResponse() by throwing UnsupportedProtocolVersion when the exception is of that kind. This fixes a problem with protocol version downgrading - before this change, the driver could not correctly downgrade the protocol version and failed with: ``` com.datastax.driver.core.exceptions.NoHostAvailableException: All host(s) tried for query failed (tried: /127.0.1.1:38223 (com.datastax.driver.core.exceptions.TransportException: [/127.0.1.1:38223] Got ERROR response message from server to an OPTIONS message: Beta version of the protocol used (5/v5-beta), but USE_BETA flag is unset)) ``` By returning a properly typed Exception, the driver can correctly retry with a lower protocol version. The reason protocol version downgrading was broken in our fork and not in upstream is that we send OPTIONS packet before STARTUP, while upstream does it in reverse. * Introduce ProtocolVersion.DEFAULT Before this change, ProtocolVersion.NEWEST_SUPPORTED was used as a default version while connecting to a cluster. DataStax Driver 3.11.0 introduced support for protocol version V5 and accordingly set NEWEST_SUPPORTED to v5. This is a problem for Scylla, as it only supports v4 protocol version causing the driver to always downgrade the protocol version upon connecting. By introducing DEFAULT (set to v4) the driver now connects with older protocol version, while maintaining support for higher protocol versions (NEWEST_SUPPORTED set to v5). * Use non-hardcoded protocol version in Parser Instead of always using ProtocolVersion.NEWEST_SUPPORTED in DataTypeClassNameParser, use the actual protocolVersion. * Add protocol version down-negotiation test Add a new integration test which creates a connection to a Cluster with NEWEST_SUPPORTED protocol version (not the default DEFAULT) to check if the driver can successfully down-negotiate the protocol version. * HostConnectionPool: log whether port-based shard awareness is used If that's not the case log the reason why. Fixes scylladb#112 Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * [maven-release-plugin] prepare release 3.11.0.0 * [maven-release-plugin] prepare for next development iteration * JAVA-2976: Protocol v5 error codes CAS_WRITE_UNKNOWN, CDC_WRITE_FAILURE not supported (3.x) (apache#1585) * Changelog updates in prep for release * Add periods for changelog messages * Update version in docs * [maven-release-plugin] prepare release 3.11.1 * [maven-release-plugin] prepare for next development iteration * Revert "[maven-release-plugin] prepare for next development iteration" This reverts commit b5fcaa7. * Revert "[maven-release-plugin] prepare release 3.11.1" This reverts commit 32be783. * [maven-release-plugin] prepare for next development iteration * Revert "[maven-release-plugin] prepare for next development iteration" This reverts commit 3e665c9. * [maven-release-plugin] prepare release 3.11.1 * Revert "[maven-release-plugin] prepare release 3.11.1" This reverts commit 11eb92e. * [maven-release-plugin] prepare release 3.11.1 * [maven-release-plugin] prepare for next development iteration * Revert "[maven-release-plugin] prepare for next development iteration" This reverts commit 48dcbbe. * Revert "[maven-release-plugin] prepare release 3.11.1" This reverts commit a4220e1. * [maven-release-plugin] prepare release 3.11.1 * [maven-release-plugin] prepare for next development iteration * SchemaParser: fetch schema tables sequentially To minimize the impact of new client connecting to a cluster with a large number of tables, this patch changes the way schema tables are fetched by the driver. The ideal solution would be to make the queries use paging but that's not currently possible because of the driver architecture. This is the first step to mitigate the problem. It makes the queries run sequentially instead of all in parallel. Also schema data is no longer fetched with single full scans. Instead we fetch keyspaces info with a full scan and then fetch schema data for each keyspace separately with a single partition reads. Keyspaces are fetched sequentially. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching tables info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching views info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching columns info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching indexes info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching UDFs info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching UDAs info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * SchemaParser: use paging for fetching UDTs info We can't use the regular paging so we emulate it with LIMIT. Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * Configuration: make it possible to turn off paging in schema queries Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> * Add port collision mitigation to TestUtils class TestUtils.findAvailablePort() will now repick a port upon choosing one that has been returned recently. Similar logic added before to CCMBridge has been removed as now it is superseded by this solution. Fixes scylladb#99. * [maven-release-plugin] prepare release 3.11.0.1 * [maven-release-plugin] prepare for next development iteration * JAVA-2984: Upgrade jackson-databind to latest 2.7.x (apache#1593) * JAVA-3008: Upgrade to Netty 4.1.75 to address CVEs (3.x) (apache#1594) * Update version in docs to 3.11.2 * Remove not ours GitHub Actions workflow Remove GitHub Actions workflow fetched from upstream DataStax repo. It wouldn't work, as it relies on secret (secrets.FOSSA_PUSH_ONLY_API_KEY) we don't have. * Fix disabling of test This is a bug present in the upstream repository. The test was supposed to be turned off, however they disabled it in a wrong way - using a JUnit annotation, but the tests are ran with TestNG. Therefore, it has to be disabled the TestNG way, not JUnit way. * Fix expected error msg in ControlConnectionTest Fix the formatting of expected error message in ControlConnectionTest. This was a bug from the upstream repository. * Fix message check in ExceptionsScassandraTest Fix message check in ExceptionsScassandraTest which was too strict. The actual error message contains additional information "In case this was generated during read repair, the consistency level is not representative of the actual consistency." at the end. Other places in this file check it using contains(). * [maven-release-plugin] prepare release 3.11.2.0 * [maven-release-plugin] prepare for next development iteration * Pin specific version of Jinja2 Before this change, the docs failed to build with the following error: ImportError: cannot import name 'environmentfilter' from 'jinja2' A fix to this problem is to explicitly pin Jinja2 version. * Add scylla-3.11.0.x and scylla-3.11.2.x to docs Add branches scylla-3.11.0.x and scylla-3.11.2.x to a list of whitelisted branches to generate documentation for. scylla-3.x is removed, as the documentation generator fails to compare 4-component version numbers (X.Y.Z.x) with 2-component version number (3.x). * Add api docs dynamic slug Fix http case * docs: update theme 1.2 docs: update dynamic slug Add back index.md Move custom slug to a separate PR Fix typo * Add Java Driver 4.x branches to documentation Add Java Driver 4.x branches to the list of generated documentation and bump the LATEST_VERSION variable. * Change each test profile running only its category Previously, "long" test profile would run tests of "unit", "short" and "long" categories. After this change, each profile only runs its own category. * Add support for parsing Scylla RC version numbers Add support for parsing Scylla RC version numbers, such as 4.3.rc5. For it to co-exist with existing VersionNumber, 4.3.rc5 is parsed into 4.3.0-rc5. * Support Scylla Enterprise in CCMBridge Add support for detecting Scylla Enterprise in CCMBridge. When a Scylla Enterprise version is detected (starts with 4-digit year), SCYLLA_PRODUCT is correctly set for scylla-ccm to start Scylla Enterprise. * Add version_fetch.py script Add a script that allows for fetching the latest version of Scylla OSS, Scylla Enterprise and Cassandra 3. * Remove unused upstream CI scripts Remove upstream CI scripts, which are unused in our project. * New version of continuous testing workflow Adds new GitHub actions workflow for continuous testing. Some of the changes: - Build is separately tested - Testing building with both Java 8 and Java 11 (Java 12+ compilers don't support building with Java 6 target version) - Formatting check is skipped in almost all steps, except for "Full verify" step - version_fetch.py script is used to fetch latest Scylla/Cassandra versions to check against - Test results are now visualized thanks to "dorny/test-reporter@v1" - Scylla Enterprise is now tested * Fix typo in 3.x tests workflow Replaces 1 ccm link, since main repo is up to date now. * Metadata.java: Add missing reserved keywords Added all the missing reserved CQL keywords. Generated using a simple script: https://github.com/Lorak-mmk/cql_reserved_keywords I didn't remove keywords that were in Metadata.java, but are not in my list, in case they were reserved in older Cassandra/Scylla versions. * Update .md docs Adds info about channels to reach out through, how to run tests against scylla. Resolves scylladb#130 * Regenerate SSL keys used in integration tests Regenerate SSL keys used in integration tests, because the certificates were expired. The new certificates have 100-year expiration date. * Properly set SSL configuration options for Scylla Extend withSSL() and withAuth() methods of CCMBridge to correctly set up SSL keys on Scylla. Convert keys to a format usable by Scylla. * Configure libssl1.0.0 for SSL tests Add installing of libssl1.0.0 on GitHub Actions for the SSL tests. Fix tc-native detection of libssl1.0.0 on Fedora systems. * Re-enable SSL integration tests Re-enable all SSL integration tests on both Cassandra and Scylla. Refs scylladb#96. * Adds reports workflow for 4.x test workflows Near identical to 3.x version (except names). * Return correct node address for scylla-jmx during testing. Stick to "localhost" for Cassandra. * docs: update theme 1.3 * Add a command for javadoc Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com> Co-authored-by: Alexandre Dutra <adutra@users.noreply.github.com> Co-authored-by: olim7t <omichallat+github@gmail.com> Co-authored-by: Madhavan <msmygit@users.noreply.github.com> Co-authored-by: Madhavan Sridharan <madhavan.sridharan@datastax.com> Co-authored-by: Sam Tunnicliffe <sam@beobal.com> Co-authored-by: Ankit Barsainya <71305148+AnkitBarsainyaPSL@users.noreply.github.com> Co-authored-by: Bret McGuire <absurdfarce@users.noreply.github.com> Co-authored-by: Laura Novich <laura@scylladb.com> Co-authored-by: Laura Novich <36125151+lauranovich@users.noreply.github.com> Co-authored-by: jianghua <jianghua@qiyi.com> Co-authored-by: Karol Baryła <karol.baryla@scylladb.com> Co-authored-by: Wojciech Bączkowski <wojciech.baczkowski@scylladb.com> Co-authored-by: Piotr Grabowski <piotr.grabowski@scylladb.com> Co-authored-by: Jeff DiNoto <jeff.dinoto@gmail.com> Co-authored-by: Piotr Jastrzebski <piotr@scylladb.com> Co-authored-by: Bret McGuire <bret.mcguire@datastax.com> Co-authored-by: Bouncheck <36934780+Bouncheck@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.