cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Paulo Motta (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-12245) initial view build can be parallel
Date Thu, 09 Nov 2017 01:34:00 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-12245?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16245074#comment-16245074
] 

Paulo Motta commented on CASSANDRA-12245:
-----------------------------------------

Getting back to this after a while, sorry about the delay. We are really close now, I think
we can wrap up after this last round of review. See follow-up below:

bq. This makes a lot of sense. I'm worried about creating thousands of tasks for large datasets
if the number of tasks is relative to the amount of data. Instead, I think we could fix the
number of partitions to the higher reasonable number of parallel tasks, something like a multiple
of the number of available processors. This would provide the desired immediate performance
improvement if the user increases the number of concurrent view builders while keeping the
number of tasks limited, independently of the amount of data. What do you think? Does it make
any sense?

Awesome! One minor thing is that we should probably only split the view build tasks at all
if the base table is larger than a given size (let's say 500MB or so?), to avoid 4 * num_processor
flushes for base tables with negligible size, WDYT?

bq. One case that we hadn't considered is that if the token ranges change or are split in
a different way when resuming a build then the progress would be lost, because ViewBuildTask
won't found any entry for the new range at system.view_builds_in_progress. This would be specially
true if we split the ranges by their data size. So, independently of how we finally split
the ranges, I think it makes sense to load all the ranges with any progress from system.view_builds_in_progress
at ViewBuilder before splitting the local ranges, create a task for those of them that are
not already finish, and then split any remaining uncovered local range. It also has the advantage
of skipping the creation of tasks for already completed ranges. What do you think?

Good idea! This will prevent a node from computing new tasks for already built subranges if
a new node joins the ring during view build. I will add a dtest to verify that scenario on
CASSANDRA-13762.

bq. I have also removed the method SystemKeyspace.beginViewBuild because I don't see any need
of saving a range without progress. Indeed, if the view build is restarted it is probably
better to don't restore the task without progress and let their tokens to be processed by
the split logic. 

+1

bq. The first wait does the opposite to _wait_for_view, it waits until there is some progress.
But we can use self._wait_for_view("ks", "t_by_v") here. Did you mean that?

Yep.

bq. Makes sense. The new test for ViewBuilderTask is here, and I've extended ViewTest.testViewBuilderResume
to run with different number of concurrent view builders.

Awesome, this looks much better now, thanks!

I had a final skim through the patch and I think we covered everything and this looks much
more robust now. Three minor things before we can close this:
- I noticed we don't stop in-progess view builds when a view is removed, would you mind adding
that?
- ViewBuildExecutor is being constructed with minThreads=1 and maxPoolSize=concurrent_materialized_view_builders,
but according to the {{DebuggableThreadPoolExecutor}}'s' [javadoc|https://github.com/apache/cassandra/blob/8b3a60b9a7dbefeecc06bace617279612ec7092d/src/java/org/apache/cassandra/concurrent/DebuggableThreadPoolExecutor.java#L33],
this will actually make the executor with size 1 since maxPoolSize is not supported by {{DebuggableThreadPoolExecutor}}
- and even if it were, new threads would only be created after the queue of the initial threads
were full (which is quite unintuitive), but we actually want the pool to have concurrent_materialized_view_builders
concurrent threads at most, so we should use the {{threadCount}} constructor instead - at
some point we should actually remove the maximumPoolSize constructors from the TPE's since
it can be quite confusing.
- In the linked dtest branch results, {{base_replica_repair_with_contention_test}} is showing
the following error:
{noformat}
ERROR [InternalResponseStage:1] 2017-10-11 11:03:16,005 CassandraDaemon.java:211 - Exception
in thread Thread[InternalResponseStage:1,5,main]
java.lang.RuntimeException: javax.management.InstanceAlreadyExistsException: org.apache.cassandra.db:type=Tables,keyspace=ks,table=t
	at org.apache.cassandra.db.ColumnFamilyStore.<init>(ColumnFamilyStore.java:484) ~[main/:na]
	at org.apache.cassandra.db.ColumnFamilyStore.createColumnFamilyStore(ColumnFamilyStore.java:639)
~[main/:na]
	at org.apache.cassandra.db.ColumnFamilyStore.createColumnFamilyStore(ColumnFamilyStore.java:613)
~[main/:na]
	at org.apache.cassandra.db.ColumnFamilyStore.createColumnFamilyStore(ColumnFamilyStore.java:604)
~[main/:na]
	at org.apache.cassandra.db.Keyspace.initCf(Keyspace.java:420) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.<init>(Keyspace.java:333) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.open(Keyspace.java:133) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.open(Keyspace.java:110) ~[main/:na]
	at org.apache.cassandra.service.StorageService.getRangesForEndpoint(StorageService.java:3467)
~[main/:na]
	at org.apache.cassandra.service.StorageService.getLocalRanges(StorageService.java:161) ~[main/:na]
	at org.apache.cassandra.db.view.ViewBuilder.build(ViewBuilder.java:125) ~[main/:na]
	at org.apache.cassandra.db.view.ViewBuilder.loadStatusAndBuild(ViewBuilder.java:93) ~[main/:na]
	at org.apache.cassandra.db.view.ViewBuilder.<init>(ViewBuilder.java:86) ~[main/:na]
	at org.apache.cassandra.db.view.View.build(View.java:201) ~[main/:na]
	at org.apache.cassandra.db.view.ViewManager.reload(ViewManager.java:132) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.<init>(Keyspace.java:335) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.open(Keyspace.java:133) ~[main/:na]
	at org.apache.cassandra.db.Keyspace.open(Keyspace.java:110) ~[main/:na]
	at org.apache.cassandra.schema.Schema.createKeyspace(Schema.java:647) ~[main/:na]
	at java.util.HashMap$Values.forEach(HashMap.java:980) ~[na:1.8.0_144]
	at java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1080) ~[na:1.8.0_144]
	at org.apache.cassandra.schema.Schema.merge(Schema.java:588) ~[main/:na]
	at org.apache.cassandra.schema.Schema.mergeAndAnnounceVersion(Schema.java:564) ~[main/:na]
	at org.apache.cassandra.schema.MigrationTask$1.response(MigrationTask.java:89) ~[main/:na]
	at org.apache.cassandra.net.ResponseVerbHandler.doVerb(ResponseVerbHandler.java:53) ~[main/:na]
	at org.apache.cassandra.net.MessageDeliveryTask.run(MessageDeliveryTask.java:72) ~[main/:na]
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) ~[na:1.8.0_144]
	at java.util.concurrent.FutureTask.run(FutureTask.java:266) ~[na:1.8.0_144]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[na:1.8.0_144]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [na:1.8.0_144]
	at org.apache.cassandra.concurrent.NamedThreadFactory.lambda$threadLocalDeallocator$0(NamedThreadFactory.java:81)
[main/:na]
	at java.lang.Thread.run(Thread.java:748) ~[na:1.8.0_144]
Caused by: javax.management.InstanceAlreadyExistsException: org.apache.cassandra.db:type=Tables,keyspace=ks,table=t
	at com.sun.jmx.mbeanserver.Repository.addMBean(Repository.java:437) ~[na:1.8.0_144]
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerWithRepository(DefaultMBeanServerInterceptor.java:1898)
~[na:1.8.0_144]
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerDynamicMBean(DefaultMBeanServerInterceptor.java:966)
~[na:1.8.0_144]
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerObject(DefaultMBeanServerInterceptor.java:900)
~[na:1.8.0_144]
	at com.sun.jmx.interceptor.DefaultMBeanServerInterceptor.registerMBean(DefaultMBeanServerInterceptor.java:324)
~[na:1.8.0_144]
	at com.sun.jmx.mbeanserver.JmxMBeanServer.registerMBean(JmxMBeanServer.java:522) ~[na:1.8.0_144]
	at org.apache.cassandra.db.ColumnFamilyStore.<init>(ColumnFamilyStore.java:479) ~[main/:na]
	... 31 common frames omitted
{noformat}

I think we could take a {{buildAllViews}} parameter on reload, and set that to false during
Keyspace initialization, since views will be build during daemon initialization and keyspace
changes anyway, WDYT?

After these nits are addressed, feel free to rebase + squash and prepare for commit and submit
a final round of CI. Thanks!

> initial view build can be parallel
> ----------------------------------
>
>                 Key: CASSANDRA-12245
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-12245
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Materialized Views
>            Reporter: Tom van der Woerdt
>            Assignee: Andrés de la Peña
>             Fix For: 4.x
>
>
> On a node with lots of data (~3TB) building a materialized view takes several weeks,
which is not ideal. It's doing this in a single thread.
> There are several potential ways this can be optimized :
>  * do vnodes in parallel, instead of going through the entire range in one thread
>  * just iterate through sstables, not worrying about duplicates, and include the timestamp
of the original write in the MV mutation. since this doesn't exclude duplicates it does increase
the amount of work and could temporarily surface ghost rows (yikes) but I guess that's why
they call it eventual consistency. doing it this way can avoid holding references to all tables
on disk, allows parallelization, and removes the need to check other sstables for existing
data. this is essentially the 'do a full repair' path



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org


Mime
View raw message