Skip to content
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

MED-100 Make read_timeout default to None, only use it if set #813

Merged
merged 3 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ jobs:
exit $?

integration-tests:
needs: build
continue-on-error: ${{ matrix.experimental }}
needs: build
strategy:
fail-fast: false
matrix:
Expand All @@ -104,6 +104,7 @@ jobs:
# IBM not included by default due to lite plan quota being easily exceeded
#it-backend: [local, s3, gcs, minio, ibm, azure]
cassandra-version: [2.2.19, 3.11.11, 4.0.0, 'github:apache/trunk']
java-version: [8.0.252, 11.0.25]
include:
# tweak the experimental flag for cassandra versions
- cassandra-version: 2.2.19
Expand All @@ -117,21 +118,35 @@ jobs:
cassandra-version: 4.0.0
it-backend: gcs
experimental: false
java-version: 11.0.25
# explicitly include tests against python 3.10 and one version of cassandra
- python-version: "3.10"
cassandra-version: 4.0.0
it-backend: gcs
experimental: false
java-version: 11.0.25
# explicitly include tests against python 3.8 and one version of cassandra
- python-version: 3.8
cassandra-version: 4.0.0
it-backend: gcs
experimental: false
java-version: 11.0.25
exclude:
# no tests against trunk
- cassandra-version: 'github:apache/trunk'
# fewer tests against cassandra 3.11.11 (exclude all but local storage backends)
- it-backend: s3
# no tests for C* 2.2 with java 11
- cassandra-version: 2.2.19
java-version: 11.0.25
# no tests for C* 3.11 with java 11
- cassandra-version: 3.11.11
java-version: 11.0.25
# no tests for C* 4.0 with java 8
- cassandra-version: 4.0.0
java-version: 8.0.252
# fewer tests against cassandra 3.11.11 (exclude all but s3 storage backends)
# we are not doing the local because it would run a scenario with mgmt-api which no longer supports 3.11
# but we still want some tests against 3.11.11, so we use s3 for at least some coverage
- it-backend: local
cassandra-version: "3.11.11"
- it-backend: gcs
cassandra-version: "3.11.11"
Expand Down Expand Up @@ -167,7 +182,7 @@ jobs:
- name: Setup Java Action
uses: actions/setup-java@v1
with:
java-version: '8.0.252'
java-version: ${{ matrix.java-version}}
architecture: x64
- name: Setup Poetry
uses: snok/install-poetry@v1
Expand Down Expand Up @@ -211,14 +226,17 @@ jobs:
# Write GCS service account credentials to a file
mkdir ~/.aws
# This fake cluster needs to be created first so that the integration tests pass in GH actions. Don't ask me why...
ccm create test_cluster -v binary:3.11.4 -n 1 --vnodes
ccm create test_cluster -v binary:${{ matrix.cassandra-version }} -n 1 --vnodes
ccm node1 updateconf 'storage_port: 7011'
ccm node1 updateconf 'concurrent_reads: 4'
ccm node1 updateconf 'concurrent_writes: 4'
ccm node1 updateconf 'concurrent_counter_writes: 4'
ccm node1 updateconf 'num_tokens: 4'
sed -i 's/#MAX_HEAP_SIZE="4G"/MAX_HEAP_SIZE="256m"/' ~/.ccm/test_cluster/node1/conf/cassandra-env.sh
sed -i 's/#HEAP_NEWSIZE="800M"/HEAP_NEWSIZE="200M"/' ~/.ccm/test_cluster/node1/conf/cassandra-env.sh
# remove the ThreadPriorityPolicy option for cases where we run with java 11
sed -i 's/-XX:ThreadPriorityPolicy=42//' ~/.ccm/test_cluster/node1/conf/jvm.options || true
sed -i 's/-XX:ThreadPriorityPolicy=42//' ~/.ccm/test_cluster/node1/conf/jvm8-server.options || true
ccm start -v
ccm showlastlog|tail -100
ccm stop
Expand Down
2 changes: 1 addition & 1 deletion medusa-example.ini
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ use_sudo_for_restore = True

;aws_cli_path = <Location of the aws cli binary if not in PATH>

; Read timeout in seconds for the storage provider.
; Read timeout in seconds for the storage provider. Not set by default.
;read_timeout = 60

[monitoring]
Expand Down
1 change: 0 additions & 1 deletion medusa/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ def _build_default_config():
'region': 'default',
'backup_grace_period_in_days': 10,
'use_sudo_for_restore': 'True',
'read_timeout': 60
}

config['logging'] = {
Expand Down
2 changes: 1 addition & 1 deletion medusa/storage/azure_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def __init__(self, config):
logging.getLogger('azure.core.pipeline.policies.http_logging_policy').setLevel(logging.WARNING)
logging.getLogger('chardet.universaldetector').setLevel(logging.WARNING)

self.read_timeout = int(config.read_timeout)
self.read_timeout = int(config.read_timeout) if 'read_timeout' in dir(config) and config.read_timeout else None

super().__init__(config)

Expand Down
6 changes: 3 additions & 3 deletions medusa/storage/google_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def __init__(self, config):

logging.getLogger('gcloud.aio.storage.storage').setLevel(logging.WARNING)

self.read_timeout = int(config.read_timeout)
self.read_timeout = int(config.read_timeout) if 'read_timeout' in dir(config) and config.read_timeout else -1

super().__init__(config)

Expand Down Expand Up @@ -158,7 +158,7 @@ async def _download_blob(self, src: str, dest: str):
stream = await self.gcs_storage.download_stream(
bucket=self.bucket_name,
object_name=object_key,
timeout=self.read_timeout if self.read_timeout is not None else -1,
timeout=self.read_timeout,
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Disabling the timeout seemed to be done by setting the value to -1 previously and now it's set to None. Does that work as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually do not know.
Since it's really tedious to find this out empirically, let's just default to -1 for GCS. The commit 3dd0a55 does that.

)
Path(file_path).parent.mkdir(parents=True, exist_ok=True)
with open(file_path, 'wb') as f:
Expand Down Expand Up @@ -243,7 +243,7 @@ async def _read_blob_as_bytes(self, blob: AbstractBlob) -> bytes:
bucket=self.bucket_name,
object_name=blob.name,
session=self.session,
timeout=self.read_timeout if self.read_timeout is not None else -1,
timeout=self.read_timeout,
)
return content

Expand Down
4 changes: 3 additions & 1 deletion medusa/storage/s3_base_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ def __init__(self, config):

self.executor = concurrent.futures.ThreadPoolExecutor(int(config.concurrent_transfers))

self.read_timeout = int(config.read_timeout) if 'read_timeout' in dir(config) and config.read_timeout else None

super().__init__(config)

def connect(self):
Expand All @@ -137,7 +139,7 @@ def connect(self):
signature_version='v4',
tcp_keepalive=True,
max_pool_connections=max_pool_size,
read_timeout=int(self.config.read_timeout),
read_timeout=self.read_timeout,
)
if self.credentials.access_key_id is not None:
self.s3_client = boto3.client(
Expand Down
11 changes: 11 additions & 0 deletions run_integration_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,17 @@ else
CASSANDRA_VERSION_FLAG="-D cassandra-version=${CASSANDRA_VERSION}"
fi

java -version 2>&1 | grep version | grep -q 11
if [ $? -ne 0 ]; then
# we're NOT having java 11, we can proceed
echo ${STORAGE_TAGS} | grep -q minio
if [ $? -eq 1 ]; then
# we cannot allow the DSE scenario with minio because in ITs it does not have the correct creds set up
STORAGE_TAGS="${STORAGE_TAGS},@dse"
fi
# do not add the dse scenario if minio is about to run
fi

if [ "$COVERAGE" == "yes" ]
then
PYTHONPATH=../.. poetry run coverage run --source='../../medusa' -m behave --stop $SCENARIO --tags=$STORAGE_TAGS $LOGGING $CASSANDRA_VERSION_FLAG
Expand Down
13 changes: 5 additions & 8 deletions tests/integration/features/integration_tests.feature
Original file line number Diff line number Diff line change
Expand Up @@ -1096,15 +1096,12 @@ Feature: Integration tests
Then I stop the DSE cluster
And I delete the DSE cluster

@local
Examples: Local storage
| storage | client encryption |
| local | without_client_encryption |
@dse
Examples: DSE Scenario
| storage | client encryption |
| local | without_client_encryption |
| s3_us_west_oregon | without_client_encryption |

@s3
Examples: S3 storage
| storage | client encryption |
| s3_us_west_oregon | without_client_encryption |

@30
Scenario Outline: Create an differential backup, corrupt it, then fix by doing another backup, and verify it
Expand Down
17 changes: 13 additions & 4 deletions tests/integration/features/steps/integration_steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ def get_client_encryption_opts(keystore_path, trustore_path):
protocol: TLS,algorithm: SunX509,store_type: JKS,cipher_suites: [{cipher_suite}]}}'"""


def tune_ccm_settings(cluster_name, custom_settings=None):
def tune_ccm_settings(cassandra_version, cluster_name, custom_settings=None):
if os.uname().sysname == "Linux":
os.popen(
"""sed -i 's/#MAX_HEAP_SIZE="4G"/MAX_HEAP_SIZE="256m"/' ~/.ccm/"""
Expand All @@ -148,6 +148,15 @@ def tune_ccm_settings(cluster_name, custom_settings=None):
+ cluster_name
+ """/node1/conf/cassandra-env.sh"""
).read()
if cassandra_version.startswith("4") or cassandra_version.startswith("5"):
jvm_options_file = "jvm8-server.options"
else:
jvm_options_file = "jvm.options"
os.popen(
"""sed -i 's/-XX:ThreadPriorityPolicy=42//' ~/.ccm/"""
+ cluster_name
+ """/node1/conf/""" + jvm_options_file
)

# sed on some macos needs `-i .bak` instead of just `-i`
if os.uname().sysname == "Darwin":
Expand Down Expand Up @@ -285,7 +294,7 @@ def _i_have_a_fresh_ccm_cluster_running(context, cluster_name, client_encryption
os.popen(update_client_encrytion_opts).read()

custom_settings = context.custom_settings if hasattr(context, 'custom_settings') else None
tune_ccm_settings(context.cluster_name, custom_settings)
tune_ccm_settings(context.cassandra_version, context.cluster_name, custom_settings)

context.session = connect_cassandra(is_client_encryption_enable)

Expand Down Expand Up @@ -335,7 +344,7 @@ def _i_have_a_fresh_ccm_cluster_with_jolokia_running(context, cluster_name, clie
)
shutil.copyfile("resources/grpc/jolokia-jvm-1.6.2-agent.jar", "/tmp/jolokia-jvm-1.6.2-agent.jar")

tune_ccm_settings(context.cluster_name)
tune_ccm_settings(context.cassandra_version, context.cluster_name)
context.session = connect_cassandra(is_client_encryption_enable)


Expand Down Expand Up @@ -370,7 +379,7 @@ def _i_have_a_fresh_ccm_cluster_with_mgmt_api_running(context, cluster_name, cli
update_client_encrytion_opts = get_client_encryption_opts(keystore_path, trustore_path)
os.popen(update_client_encrytion_opts).read()

tune_ccm_settings(context.cluster_name)
tune_ccm_settings(context.cassandra_version, context.cluster_name)
context.session = connect_cassandra(is_client_encryption_enable)
# stop the node via CCM as it needs to be started by the Management API
os.popen(CCM_STOP).read()
Expand Down
1 change: 1 addition & 0 deletions tests/resources/config/medusa-s3_us_west_oregon.ini
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ concurrent_transfers = 16
backup_grace_period_in_days = 0
max_backup_count = 1
region = us-west-2
read_timeout = 60

[monitoring]
monitoring_provider = local
1 change: 1 addition & 0 deletions tests/storage/abstract_storage_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class AttributeDict(dict):
__slots__ = ()
__getattr__ = dict.__getitem__
__setattr__ = dict.__setitem__
__dict__ = dict.__dict__


class TestAbstractStorage(AbstractStorage):
Expand Down
Loading