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

Kerberized kafka error #48393

Merged
merged 2 commits into from
Apr 5, 2023
Merged

Kerberized kafka error #48393

merged 2 commits into from
Apr 5, 2023

Conversation

Felixoid
Copy link
Member

@Felixoid Felixoid commented Apr 4, 2023

Changelog category (leave one):

  • Build/Testing/Packaging Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

With the current approach, all ports are calculated at the beginning and could overlap or even be highjacked, see the report for port is already allocated. It's possibly the reason for #45368.

@robot-ch-test-poll3 robot-ch-test-poll3 added the pr-build Pull request with build/testing/packaging improvement label Apr 4, 2023
@yakov-olkhovskiy
Copy link
Member

I'm afraid this approach will not guarantee reliable solution - port will be released between acquisition and real use, and it heavily depends on how we use these properties - for example we have code like this:

def kafka_produce(kafka_cluster, topic, messages, timestamp=None, retries=15):
    logging.debug(
        "kafka_produce server:{}:{} topic:{}".format(
            "localhost", kafka_cluster.kafka_port, topic
        )
    )
    producer = get_kafka_producer(
        kafka_cluster.kafka_port, producer_serializer, retries
    )
    for message in messages:
        producer.send(topic=topic, value=message, timestamp_ms=timestamp)
        producer.flush()

notice kafka_port is used in logging first...

@yakov-olkhovskiy yakov-olkhovskiy self-assigned this Apr 4, 2023
@Felixoid
Copy link
Member Author

Felixoid commented Apr 5, 2023

I agree, it's better, but not 100% reliable.

It's much better at least by reducing the time between assigning *_port and using it. Another point, we don't collect every possible port at __init__ anymore, and reserve them only on demand

@Felixoid Felixoid merged commit 14d6373 into master Apr 5, 2023
138 of 139 checks passed
@Felixoid Felixoid deleted the kerberized-kafka-error branch April 5, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-build Pull request with build/testing/packaging improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants