Skip to content

IGNITE-13492: Added snapshot test to ducktests#8575

Merged
anton-vinogradov merged 55 commits into
apache:ignite-ducktapefrom
Sega76:IGNITE-13492v1
Feb 19, 2021
Merged

IGNITE-13492: Added snapshot test to ducktests#8575
anton-vinogradov merged 55 commits into
apache:ignite-ducktapefrom
Sega76:IGNITE-13492v1

Conversation

@Sega76
Copy link
Copy Markdown
Contributor

@Sega76 Sega76 commented Dec 14, 2020

Thank you for submitting the pull request to the Apache Ignite.

In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

Notes

If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.

"""
return str(node.account.ssh_client.exec_command("sudo iptables -L -n")[1].read(), sys.getdefaultencoding())

def restart(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

make it private

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

made a private rotate_log

node.account.ssh(f'if [ -e {self.STDOUT_STDERR_CAPTURE} ];'
f'then '
f'cd {self.LOGS_DIR};'
f'N=`ls | grep log | wc -l`;'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use grep with full pattern patching, smth like "^console_?[0-9]*.log$". As string "log" may match different log files.

Alternative is using service counter variable that incremented within restart method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

when starting different services on the same nodes (for example, when testing the PDS), we will not be able to use counter the service.
So I think the counter is a bad idea.

…92v1

# Conflicts:
#	modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py
#	modules/ducktests/tests/ignitetest/services/utils/ignite_persistence.py
…92v1

# Conflicts:
#	modules/ducktests/tests/ignitetest/services/utils/control_utility.py
#	modules/ducktests/tests/ignitetest/services/utils/ignite_persistence.py
@Override public void run(JsonNode jNode) {
String cacheName = jNode.get("cacheName").asText();

long size = jNode.get("size").asLong();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is "size" when we have "dataSize"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On, now we have "amount". Amount of what?


int dataSize = jNode.get("dataSize").asInt();

markInitialized();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the reason to have a dedicated "initialized" state?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

File "/opt/ignite-dev/modules/ducktests/tests/ignitetest/tests/snapshot_test.py", line 87, in snapshot_test
loader.run()
File "/usr/local/lib/python3.7/dist-packages/ducktape/services/service.py", line 322, in run
self.start()
File "/opt/ignite-dev/modules/ducktests/tests/ignitetest/services/utils/ignite_aware.py", line 85, in start
self.await_started()
File "/opt/ignite-dev/modules/ducktests/tests/ignitetest/services/ignite_app.py", line 51, in await_started
self.__check_status("IGNITE_APPLICATION_INITIALIZED", timeout=self.startup_timeout_sec)

need "initialized" state

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

image
Line 87 is empty.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let me ask again,
what is the reason to have dedicated "initialized" and "finished" states?
Why not only the markSyncExecutionComplete() used?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if something goes wrong, during operation, then we will consider it during initialization, I think this is not correct


def idle_verify_dump(self, node=None):
"""
Idle verify dump.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Node" param description missed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"Node where will be dump file"
Could you please specify an action in this sentence?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

indexed_types=['java.util.UUID', 'byte[]'])]
)

num_nodes = len(self.test_context.cluster)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

any reason to have this variable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we can run this test by passing a parameter through globals

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

any reason to have a dedicated variable?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed


num_nodes = len(self.test_context.cluster)

service = IgniteService(self.test_context, ignite_config, num_nodes=num_nodes - 1, startup_timeout_sec=180)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why it may take so long time to startup?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

for local tests like in PmeFreeSwitchTest

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please answer the question I asked?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Startup takes the same amount of time as in other tests with PDS.
Time increased for local startup.

Comment thread modules/ducktests/tests/ignitetest/tests/snapshot_test.py Outdated
/**
* Loading random uuids to cache.
*/
public class UuidDataLoaderApplication extends IgniteAwareApplication {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why it streams UUID as a key?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

we may run it repeatedly, key should be different

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't see tests where it runs repeatedly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

SnapshotTest
we run it repeatedly

node.account.kill_java_processes(self.APP_SERVICE_CLASS, clean_shutdown=False, allow_fail=True)
node.account.ssh("rm -rf -- %s" % self.persistent_root, allow_fail=False)

def rename_database(self, new_name: str):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it performs "mv" but has the "rename*" name...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"move" implies getting a new path. we rename the base folder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems you changed mv to cp, what the reason to perform a so heavy operation?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed this method

params={
"cacheName": self.CACHE_NAME,
"interval": 500_000,
"dataSizeKB": 1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

valueSizeKb is better imho

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@anton-vinogradov anton-vinogradov merged commit 8fa155c into apache:ignite-ducktape Feb 19, 2021
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.

4 participants